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

feat: Update STS web identity credentials provider & add integration test #1327

Merged
merged 12 commits into from
Feb 9, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ func addIntegrationTestTarget(_ name: String) {
]
case "AWSS3":
additionalDependencies = ["AWSSSOAdmin"]
case "AWSSTS":
additionalDependencies = ["AWSIAM", "AWSCognitoIdentity"]
default:
break
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
//
// Copyright Amazon.com Inc. or its affiliates.
// All Rights Reserved.
//
// SPDX-License-Identifier: Apache-2.0
//

import XCTest
import AWSSTS
import AWSIAM
import AWSCognitoIdentity
import ClientRuntime
import AWSClientRuntime
import Foundation

/// Tests STS web identity credentials provider using STS::getCallerIdentity.
class STSWebIdentityCredentialsProviderTests: XCTestCase {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move helpers and role information to a separate file so that this file is easier to read

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather keep stuff for this integration test confined to one file. I'll go ahead and refactor stuff into more helper methods and locate them all at bottom of file for better readability.

private let region = "us-west-2"
private var oidcToken: String!
// File path for the cached OIDC token.
private var tokenFilePath: String!

// STS client with only the STS web identity credentials provider configured.
private var webIdentityStsClient: STSClient!
private var stsConfig: STSClient.STSClientConfiguration!

// Used to create identity pools and fetch cognito ID & OIDC token from it.
private var cognitoIdentityClient: CognitoIdentityClient!
// Regular STS client; used to fetch the account ID used in fetching cognito ID.
private var stsClient: STSClient!
// Cognito identity pool name & ID
private let identityPoolName = "aws-sts-integration-test-\(UUID().uuidString.split(separator: "-").first!.lowercased())"
private var identityPoolId: String!

// Used to create temporary role assumed by STS web identity credentials provider.
private var iamClient: IAMClient!
// Role properties
private let roleName = "aws-sts-integration-test-\(UUID().uuidString.split(separator: "-").first!.lowercased())"
private let roleSessionName = "aws-sts-integration-test-\(UUID().uuidString.split(separator: "-").first!.lowercased())"
private var roleArn: String!

// JSON assume role policy
private let assumeRolePolicy = """
{
"Version": "2012-10-17",
"Statement": [
{
"Sid": "",
"Effect": "Allow",
"Principal": {
"Federated": "cognito-identity.amazonaws.com"
},
"Action": [
"sts:AssumeRoleWithWebIdentity"
],
"Condition": {
"ForAnyValue:StringLike": {
"cognito-identity.amazonaws.com:amr": "unauthenticated"
}
}
}
]
}
"""
// Identity policy name & JSON identity policy
private let identityPolicyName = "allow-STS-getCallerIdentity"
private let roleIdentityPolicy = """
{
"Version": "2012-10-17",
"Statement": [
{
"Sid": "",
"Effect": "Allow",
"Action": "sts:GetCallerIdentity",
"Resource": "*"
}
]
}
"""

override func setUp() async throws {
try await super.setUp()

// Create the role to be assumed in exchange for web identity token
iamClient = try IAMClient(region: region)
let role = try await iamClient.createRole(input: CreateRoleInput(
assumeRolePolicyDocument: assumeRolePolicy,
roleName: roleName
))
roleArn = role.role?.arn

// This wait is necessary for role creation to propagate everywhere
let seconds = 10
let NSEC_PER_SEC = 1_000_000_000
try await Task.sleep(nanoseconds: UInt64(seconds * NSEC_PER_SEC))
Comment on lines +125 to +128
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't introduce anything non-deterministic in our integration tests if we can help it. A better approach would be to poll IAM to see if the role was created successfully.

Example using getRole:

func isRoleReady(iamClient: IAMClient, roleName: String) async -> Bool {
    do {
        let response = try await iamClient.getRole(input: GetRoleInput(roleName: roleName))
        return response.role != nil
    } catch {
        return false
    }
}

Then you can setup a function that calls isRoleReady every X seconds using Task.sleep until a configured timeout (if after X seconds/minutes the role is still not there, throw an error).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It definitely would be ideal to have some polling mechanism like the one you suggested instead of a fixed wait behavior, but getRole doesn't serve our purpose here (nor does any other IAM API). That's because changes to IAM entities become available to IAM API immediately BUT it takes time for that info to propagate everywhere. With 7 second wait it reliably passed every time on local machine, so I made it 10 just to be safe. Here's a relevant stackoverflow discussion if you'd like to read more about it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also see if there is a waiter defined on the service for the resource you are waiting on.

Check the Kinesis integration test: it uses a predefined waiter and manual waiting in two stages of the test to ensure conditions are met before proceeding.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried the waiters and they don't serve the polling purpose in this instance for the same reasons explained above, as expected. Unless IAM provides some sort of API where it shows IAM identity's propagation status, I don't think it's possible to poll this info.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, we can try this live in CI for a while, but we'll have to revisit this test if it causes spurious failures in practice.

With the size & complexity of our test suite, we can't afford even a small number of flaky tests.


// Attach identity policy to role
_ = try await iamClient.putRolePolicy(input: PutRolePolicyInput(
policyDocument: roleIdentityPolicy,
policyName: identityPolicyName,
roleName: roleName
))

// Create the Cognito identity pool that allows unauthenticated identities
cognitoIdentityClient = try CognitoIdentityClient(region: region)
let identityPool = try await cognitoIdentityClient.createIdentityPool(
input: CreateIdentityPoolInput(
allowUnauthenticatedIdentities: true,
identityPoolName: identityPoolName
))
identityPoolId = identityPool.identityPoolId

// Get Cognito ID from Cognito identity pool
stsClient = try STSClient(region: region)
let accountId = try await stsClient.getCallerIdentity(input: GetCallerIdentityInput()).account
let cognitoId = try await cognitoIdentityClient.getId(input: GetIdInput(
accountId: accountId, identityPoolId: identityPoolId
))

// Get OIDC token from Cognito identity pool using Cognito ID
oidcToken = try await cognitoIdentityClient.getOpenIdToken(
input: GetOpenIdTokenInput(identityId: cognitoId.identityId)
).token
// Save OIDC token to a file then save filepath
try saveTokenIntoFile()

// Construct STS web identity credentials provider
let webIdentityCredentialsProvider = try STSWebIdentityCredentialsProvider(
region: region,
roleArn: roleArn,
roleSessionName: roleSessionName,
tokenFilePath: tokenFilePath
)

// Construct STS config with STS web identity credentials provider
stsConfig = try await STSClient.STSClientConfiguration(
credentialsProvider: webIdentityCredentialsProvider,
region: region
)

// Construct STS client with just the...
// ...STS web identity credentials provider configured
webIdentityStsClient = STSClient(config: stsConfig)
}

override func tearDown() async throws {
// Delete inline identity policy of the role
let policyName = try await iamClient.listRolePolicies(input: ListRolePoliciesInput(roleName: roleName)).policyNames
_ = try await iamClient.deleteRolePolicy(input: DeleteRolePolicyInput(
policyName: policyName?[0], roleName: roleName
))
// Delete role
_ = try await iamClient.deleteRole(input: DeleteRoleInput(roleName: roleName))
// Delete Cognito identity pool
_ = try await cognitoIdentityClient.deleteIdentityPool(input: DeleteIdentityPoolInput(identityPoolId: identityPoolId))
// Delete token file
try deleteTokenFile()
}

// Helper methods for saving & deleting token file
private func saveTokenIntoFile() throws {
tokenFilePath = NSHomeDirectory() + "/token.txt"
Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to use FileManager.default.temporaryDirectory instead of NSHomeDirectory()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed as suggested.

let tokenData = oidcToken.data(using: String.Encoding.utf8)
let fileCreated = FileManager.default.createFile(
atPath: tokenFilePath, contents: tokenData, attributes: nil
)
if !fileCreated {
throw TokenFileError.TokenFileCreationFailed("Failed to create token text file.")
}
}
private func deleteTokenFile() throws {
do {
try FileManager.default.removeItem(atPath: tokenFilePath)
} catch {
throw TokenFileError.TokenFileDeletionFailed("Failed to delete token text file.")
}
}

// Confirm STS web identity credentials provider gave valid credentials
// ...by checking response was returned successfully.
Copy link
Contributor

Choose a reason for hiding this comment

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

Make use of Swift's multi-line comments instead of ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed as suggested.

func testGetCallerIdentity() async throws {
let response = try await webIdentityStsClient.getCallerIdentity(
input: GetCallerIdentityInput()
)

// Ensure returned caller info aren't nil
let account = try XCTUnwrap(response.account)
let userId = try XCTUnwrap(response.userId)
let arn = try XCTUnwrap(response.arn)

// Ensure returned caller info aren't empty strings
XCTAssertNotEqual(account, "")
XCTAssertNotEqual(userId, "")
XCTAssertNotEqual(arn, "")
}
}

enum TokenFileError: Error {
case TokenFileCreationFailed(String)
case TokenFileDeletionFailed(String)
}

enum IdentityPoolStatus {
case INITIALIZING
case DONE
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,17 @@ public struct STSWebIdentityCredentialsProvider: CredentialsSourcedByCRT {
/// - Parameters:
/// - configFilePath: The path to the configuration file to use. If not provided it will be resolved internally via the `AWS_CONFIG_FILE` environment variable or defaulted to `~/.aws/config` if not configured.
/// - credentialsFilePath: The path to the shared credentials file to use. If not provided it will be resolved internally via the `AWS_SHARED_CREDENTIALS_FILE` environment variable or defaulted `~/.aws/credentials` if not configured.
/// - region: (Optional) region override.
/// - roleArn: (Optional) roleArn override.
/// - roleSessionName: (Optional) roleSessionName override.
/// - tokenFilePath: (Optional) tokenFilePath override.
public init(
configFilePath: String? = nil,
credentialsFilePath: String? = nil
credentialsFilePath: String? = nil,
region: String? = nil,
roleArn: String? = nil,
roleSessionName: String? = nil,
tokenFilePath: String? = nil
) throws {
let fileBasedConfig = try CRTFileBasedConfiguration(
configFilePath: configFilePath,
Expand All @@ -38,6 +46,10 @@ public struct STSWebIdentityCredentialsProvider: CredentialsSourcedByCRT {
bootstrap: SDKDefaultIO.shared.clientBootstrap,
tlsContext: SDKDefaultIO.shared.tlsContext,
fileBasedConfiguration: fileBasedConfig,
region: region,
roleArn: roleArn,
roleSessionName: roleSessionName,
tokenFilePath: tokenFilePath,
shutdownCallback: nil
))
}
Expand Down
2 changes: 1 addition & 1 deletion scripts/integration-test-sdk.properties
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
# Only include services needed for running integration tests
onlyIncludeModels=kinesis.2013-12-02,s3.2006-03-01,sso-admin.2020-07-20,translate.2017-07-01,sqs.2012-11-05,mediaconvert.2017-08-29,sts.2011-06-15
onlyIncludeModels=kinesis.2013-12-02,s3.2006-03-01,sso-admin.2020-07-20,translate.2017-07-01,sqs.2012-11-05,mediaconvert.2017-08-29,sts.2011-06-15,cognito-identity.2014-06-30,iam.2010-05-08
Loading