Skip to content

Commit 5e71bf2

Browse files
committed
fix(storage) Storage.uploadFile exits early on access denied
1 parent d20ac4e commit 5e71bf2

File tree

3 files changed

+108
-0
lines changed

3 files changed

+108
-0
lines changed

AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadFileOperation.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,17 @@ class AWSS3StorageUploadFileOperation: AmplifyInProcessReportingOperation<
8484
return
8585
}
8686

87+
// This check was added because, at the time of this writing, AWS SDK
88+
// failed silently on access denied.
89+
if FileManager.default.fileExists(atPath: request.local.path) {
90+
guard FileManager.default.isReadableFile(atPath: request.local.path) else {
91+
dispatch(StorageError.accessDenied("Access to local file denied: \(request.local.path)",
92+
"Please ensure that \(request.local) is readable"))
93+
finish()
94+
return
95+
}
96+
}
97+
8798
let uploadSize: UInt64
8899
do {
89100
uploadSize = try StorageRequestUtils.getSize(request.local)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
////
2+
// Copyright Amazon.com Inc. or its affiliates.
3+
// All Rights Reserved.
4+
//
5+
// SPDX-License-Identifier: Apache-2.0
6+
//
7+
8+
@testable import Amplify
9+
@testable import AmplifyTestCommon
10+
@testable import AWSPluginsTestCommon
11+
@testable import AWSS3StoragePlugin
12+
@testable import AWSPluginsCore
13+
14+
import AWSS3
15+
import XCTest
16+
17+
final class AWSS3StorageUploadFileOperationTests2: AWSS3StorageOperationTestBase {
18+
19+
override func setUp() {
20+
super.setUp()
21+
self.continueAfterFailure = false
22+
}
23+
24+
/// - Given: A file with no read permissions from the user
25+
/// - When: An attempt is made to upload it
26+
/// - Then: An accessDenied error is returned before attempting proceed further
27+
func testAccessDenied() throws {
28+
let path = NSTemporaryDirectory().appending(UUID().uuidString)
29+
FileManager.default.createFile(atPath: path,
30+
contents: Data(UUID().uuidString.utf8),
31+
attributes: [FileAttributeKey.posixPermissions: 000])
32+
defer {
33+
try? FileManager.default.removeItem(atPath: path)
34+
}
35+
36+
let url = URL(fileURLWithPath: path)
37+
let key = (path as NSString).lastPathComponent
38+
let options = StorageUploadFileRequest.Options(accessLevel: .protected)
39+
let request = StorageUploadFileRequest(key: key, local: url, options: options)
40+
41+
let progressExpectation = expectation(description: "progress")
42+
progressExpectation.isInverted = true
43+
let progressListner: ProgressListener = { _ in progressExpectation.fulfill() }
44+
45+
let resultExpectation = expectation(description: "result")
46+
let resultListener: StorageUploadFileOperation.ResultListener = { result in
47+
defer {
48+
resultExpectation.fulfill()
49+
}
50+
switch result {
51+
case .failure(let error):
52+
guard case .accessDenied(let description, let recommendation, _) = error else {
53+
XCTFail("Should have failed with validation error")
54+
return
55+
}
56+
XCTAssertEqual(description, "Access to local file denied: \(path)")
57+
XCTAssertEqual(recommendation, "Please ensure that \(url) is readable")
58+
case .success:
59+
XCTFail("Expecting an error but got success")
60+
}
61+
}
62+
63+
let operation = AWSS3StorageUploadFileOperation(request,
64+
storageConfiguration: testStorageConfiguration,
65+
storageService: mockStorageService,
66+
authService: mockAuthService,
67+
progressListener: progressListner,
68+
resultListener: resultListener)
69+
operation.start()
70+
71+
wait(for: [progressExpectation, resultExpectation], timeout: 1)
72+
XCTAssertTrue(operation.isFinished)
73+
}
74+
}

AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginNegativeTests.swift

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,29 @@ class AWSS3StoragePluginNegativeTests: AWSS3StoragePluginTestBase {
9292
XCTAssertEqual(description, StorageErrorConstants.localFileNotFound.errorDescription)
9393
}
9494

95+
/// Given: An unreadable file
96+
/// When: An attempt to upload it is made
97+
/// Then: A StorageError.accessDenied error is propagated to the caller
98+
func testUploadUnreadableFile() async throws {
99+
let key = UUID().uuidString
100+
let path = NSTemporaryDirectory() + key + ".tmp"
101+
FileManager.default.createFile(atPath: path,
102+
contents: Data(key.utf8),
103+
attributes: [FileAttributeKey.posixPermissions: 000])
104+
defer {
105+
try? FileManager.default.removeItem(atPath: path)
106+
}
107+
108+
let url = URL(fileURLWithPath: path)
109+
do {
110+
_ = try await Amplify.Storage.uploadFile(key: key, local: url, options: nil).value
111+
} catch StorageError.accessDenied(let description, let recommendation, let underlyingError) {
112+
XCTAssertEqual(description, "Access to local file denied: \(path)")
113+
XCTAssertEqual(recommendation, "Please ensure that \(url) is readable")
114+
XCTAssertNil(underlyingError)
115+
}
116+
}
117+
95118
// TODO: possibly after understanding content-type
96119
// func testPutWithInvalidContentType() {
97120
//

0 commit comments

Comments
 (0)