Skip to content

Commit 950c403

Browse files
committed
fix: address PR review feedback for DynamoDB support
1 parent 4dd4d02 commit 950c403

9 files changed

Lines changed: 18873 additions & 18359 deletions

File tree

Plugins/DynamoDBDriverPlugin/DynamoDBConnection.swift

Lines changed: 47 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,21 @@ final class DynamoDBConnection: @unchecked Sendable {
256256
self.region = config.additionalFields["awsRegion"] ?? "us-east-1"
257257

258258
if let customEndpoint = config.additionalFields["awsEndpointUrl"], !customEndpoint.isEmpty {
259-
self.endpointUrl = customEndpoint
259+
if customEndpoint.lowercased().hasPrefix("http://") {
260+
let loopbackHosts: Set<String> = ["localhost", "127.0.0.1", "::1"]
261+
let isLoopback = URL(string: customEndpoint).flatMap(\.host).map {
262+
loopbackHosts.contains($0.lowercased())
263+
} ?? false
264+
if isLoopback {
265+
self.endpointUrl = customEndpoint
266+
} else {
267+
let upgraded = "https://" + customEndpoint.dropFirst("http://".count)
268+
Self.logger.warning("Insecure endpoint for non-loopback host, upgrading to HTTPS")
269+
self.endpointUrl = upgraded
270+
}
271+
} else {
272+
self.endpointUrl = customEndpoint
273+
}
260274
} else {
261275
self.endpointUrl = "https://dynamodb.\(region).amazonaws.com"
262276
}
@@ -340,7 +354,8 @@ final class DynamoDBConnection: @unchecked Sendable {
340354
expressionAttributeValues: [String: DynamoDBAttributeValue],
341355
limit: Int? = nil,
342356
exclusiveStartKey: [String: DynamoDBAttributeValue]? = nil,
343-
scanIndexForward: Bool = true
357+
scanIndexForward: Bool = true,
358+
select: String? = nil
344359
) async throws -> QueryResponse {
345360
var body: [String: Any] = [
346361
"TableName": tableName,
@@ -354,6 +369,9 @@ final class DynamoDBConnection: @unchecked Sendable {
354369
body["ExclusiveStartKey"] = try encodedAttributeMap(startKey)
355370
}
356371
body["ScanIndexForward"] = scanIndexForward
372+
if let select = select {
373+
body["Select"] = select
374+
}
357375
return try await request(target: "DynamoDB_20120810.Query", body: body)
358376
}
359377

@@ -402,7 +420,8 @@ final class DynamoDBConnection: @unchecked Sendable {
402420

403421
let (data, response) = try await withCheckedThrowingContinuation {
404422
(continuation: CheckedContinuation<(Data, URLResponse), Error>) in
405-
let task = urlSession.dataTask(with: urlRequest) { data, response, error in
423+
let task = urlSession.dataTask(with: urlRequest) { [weak self] data, response, error in
424+
self?.lock.withLock { self?._currentTask = nil }
406425
if let error {
407426
if (error as? URLError)?.code == .cancelled {
408427
continuation.resume(throwing: DynamoDBError.requestCancelled)
@@ -417,6 +436,7 @@ final class DynamoDBConnection: @unchecked Sendable {
417436
}
418437
continuation.resume(returning: (data, response))
419438
}
439+
self.lock.withLock { self._currentTask = task }
420440
task.resume()
421441
}
422442

@@ -443,8 +463,7 @@ final class DynamoDBConnection: @unchecked Sendable {
443463
let decoded = try JSONDecoder().decode(T.self, from: data)
444464
return decoded
445465
} catch {
446-
let bodyStr = String(data: data, encoding: .utf8) ?? "<binary>"
447-
Self.logger.error("Decode failed for \(target): \(error.localizedDescription)\nBody: \(bodyStr)")
466+
Self.logger.error("Decode failed for \(target): responseLength=\(data.count), error=\(error.localizedDescription)")
448467
throw DynamoDBError.invalidResponse("Failed to decode response: \(error.localizedDescription)")
449468
}
450469
}
@@ -575,9 +594,7 @@ final class DynamoDBConnection: @unchecked Sendable {
575594
let secretAccessKey = config.additionalFields["awsSecretAccessKey"] ?? config.password
576595
let sessionToken = config.additionalFields["awsSessionToken"]
577596

578-
NSLog("[DynamoDB] Resolved credentials - accessKeyId: '%@', secretKey length: %d, region: '%@', endpoint: '%@'",
579-
accessKeyId, secretAccessKey.count, region, endpointUrl)
580-
NSLog("[DynamoDB] additionalFields keys: %@", Array(config.additionalFields.keys).sorted().description)
597+
Self.logger.debug("Resolved credentials — credentialSource: accessKey, region: \(self.region)")
581598

582599
guard !accessKeyId.isEmpty, !secretAccessKey.isEmpty else {
583600
throw DynamoDBError.authFailed("Access Key ID and Secret Access Key are required")
@@ -611,8 +628,10 @@ final class DynamoDBConnection: @unchecked Sendable {
611628
}
612629
guard currentProfile == profileName else { continue }
613630

614-
let parts = trimmed.components(separatedBy: "=").map { $0.trimmingCharacters(in: .whitespaces) }
615-
guard parts.count >= 2 else { continue }
631+
let parts = trimmed.split(separator: "=", maxSplits: 1).map {
632+
$0.trimmingCharacters(in: .whitespaces)
633+
}
634+
guard parts.count == 2 else { continue }
616635

617636
switch parts[0] {
618637
case "aws_access_key_id":
@@ -638,30 +657,35 @@ final class DynamoDBConnection: @unchecked Sendable {
638657
}
639658

640659
private func resolveSsoCredentials() throws -> AWSCredentials {
641-
let ssoCachePath = NSString("~/.aws/sso/cache").expandingTildeInPath
660+
let cliCachePath = NSString("~/.aws/cli/cache").expandingTildeInPath
642661

643-
guard let enumerator = FileManager.default.enumerator(atPath: ssoCachePath) else {
644-
throw DynamoDBError.authFailed("Cannot read ~/.aws/sso/cache/")
662+
guard let enumerator = FileManager.default.enumerator(atPath: cliCachePath) else {
663+
throw DynamoDBError.authFailed("Cannot read ~/.aws/cli/cache/")
645664
}
646665

647666
var latestToken: (accessKeyId: String, secretAccessKey: String, sessionToken: String, expiresAt: Date)?
648667

649668
while let fileName = enumerator.nextObject() as? String {
650669
guard fileName.hasSuffix(".json") else { continue }
651-
let filePath = (ssoCachePath as NSString).appendingPathComponent(fileName)
670+
let filePath = (cliCachePath as NSString).appendingPathComponent(fileName)
652671
guard let data = FileManager.default.contents(atPath: filePath) else { continue }
653672
guard let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any] else { continue }
654673

655-
guard let accessKeyId = json["accessKeyId"] as? String,
656-
let secretAccessKey = json["secretAccessKey"] as? String,
657-
let sessionToken = json["sessionToken"] as? String,
658-
let expiresAtStr = json["expiresAt"] as? String
674+
guard let accessKeyId = json["AccessKeyId"] as? String,
675+
let secretAccessKey = json["SecretAccessKey"] as? String,
676+
let sessionToken = json["SessionToken"] as? String
659677
else { continue }
660678

661-
let formatter = ISO8601DateFormatter()
662-
formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds]
663-
guard let expiresAt = formatter.date(from: expiresAtStr) ?? ISO8601DateFormatter().date(from: expiresAtStr)
664-
else { continue }
679+
let expiresAt: Date
680+
if let expiresAtStr = json["Expiration"] as? String {
681+
let formatter = ISO8601DateFormatter()
682+
formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds]
683+
guard let parsed = formatter.date(from: expiresAtStr) ?? ISO8601DateFormatter().date(from: expiresAtStr)
684+
else { continue }
685+
expiresAt = parsed
686+
} else {
687+
continue
688+
}
665689

666690
guard expiresAt > Date() else { continue }
667691

@@ -672,7 +696,7 @@ final class DynamoDBConnection: @unchecked Sendable {
672696

673697
guard let token = latestToken else {
674698
throw DynamoDBError.authFailed(
675-
"No valid SSO credentials found in ~/.aws/sso/cache/. Run 'aws sso login' first."
699+
"No valid SSO credentials found in ~/.aws/cli/cache/. Run 'aws sso login' first."
676700
)
677701
}
678702

Plugins/DynamoDBDriverPlugin/DynamoDBPlugin.swift

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ final class DynamoDBPlugin: NSObject, TableProPlugin, DriverPlugin {
9090
placeholder: "AKIA...",
9191
section: .authentication
9292
),
93+
// TODO: .secure fields stored in additionalFields (plain JSON), not Keychain — needs migration
9394
ConnectionField(
9495
id: "awsSecretAccessKey",
9596
label: String(localized: "Secret Access Key"),
@@ -114,24 +115,9 @@ final class DynamoDBPlugin: NSObject, TableProPlugin, DriverPlugin {
114115
ConnectionField(
115116
id: "awsRegion",
116117
label: String(localized: "AWS Region"),
118+
placeholder: "us-east-1",
117119
defaultValue: "us-east-1",
118-
fieldType: .dropdown(options: [
119-
.init(value: "us-east-1", label: "US East (N. Virginia)"),
120-
.init(value: "us-east-2", label: "US East (Ohio)"),
121-
.init(value: "us-west-1", label: "US West (N. California)"),
122-
.init(value: "us-west-2", label: "US West (Oregon)"),
123-
.init(value: "eu-west-1", label: "Europe (Ireland)"),
124-
.init(value: "eu-west-2", label: "Europe (London)"),
125-
.init(value: "eu-central-1", label: "Europe (Frankfurt)"),
126-
.init(value: "ap-northeast-1", label: "Asia Pacific (Tokyo)"),
127-
.init(value: "ap-southeast-1", label: "Asia Pacific (Singapore)"),
128-
.init(value: "ap-southeast-2", label: "Asia Pacific (Sydney)"),
129-
.init(value: "ap-south-1", label: "Asia Pacific (Mumbai)"),
130-
.init(value: "sa-east-1", label: "South America (São Paulo)"),
131-
.init(value: "ca-central-1", label: "Canada (Central)"),
132-
.init(value: "me-south-1", label: "Middle East (Bahrain)"),
133-
.init(value: "af-south-1", label: "Africa (Cape Town)")
134-
]),
120+
fieldType: .text,
135121
section: .authentication
136122
),
137123
ConnectionField(

0 commit comments

Comments
 (0)