Skip to content

Commit e4fe50d

Browse files
committed
Remove addOrUpdate from protocol; append to .netrc file instead of overwrite
1 parent 101f7c7 commit e4fe50d

File tree

2 files changed

+77
-75
lines changed

2 files changed

+77
-75
lines changed

Sources/Basics/AuthorizationProvider.swift

Lines changed: 27 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ import TSCBasic
1818
import TSCUtility
1919

2020
public protocol AuthorizationProvider {
21-
mutating func addOrUpdate(for url: Foundation.URL, user: String, password: String, callback: @escaping (Result<Void, Error>) -> Void)
22-
2321
func authentication(for url: Foundation.URL) -> (user: String, password: String)?
2422
}
2523

@@ -60,7 +58,7 @@ public struct NetrcAuthorizationProvider: AuthorizationProvider {
6058

6159
private var underlying: TSCUtility.Netrc?
6260

63-
private var machines: [TSCUtility.Netrc.Machine] {
61+
var machines: [TSCUtility.Netrc.Machine] {
6462
self.underlying?.machines ?? []
6563
}
6664

@@ -71,36 +69,36 @@ public struct NetrcAuthorizationProvider: AuthorizationProvider {
7169
}
7270

7371
public mutating func addOrUpdate(for url: Foundation.URL, user: String, password: String, callback: @escaping (Result<Void, Error>) -> Void) {
74-
guard let machineName = self.machineName(for: url) else {
72+
guard let machine = url.authenticationID else {
7573
return callback(.failure(AuthorizationProviderError.invalidURLHost))
7674
}
77-
let machine = TSCUtility.Netrc.Machine(name: machineName, login: user, password: password)
78-
79-
var machines = [TSCUtility.Netrc.Machine]()
80-
var hasExisting = false
81-
82-
self.machines.forEach {
83-
if $0.name.lowercased() != machineName {
84-
machines.append($0)
85-
} else if !hasExisting {
86-
// Update existing entry and retain one copy only
87-
machines.append(machine)
88-
hasExisting = true
89-
}
90-
}
9175

92-
// New entry
93-
if !hasExisting {
94-
machines.append(machine)
76+
// Same entry already exists, no need to add or update
77+
guard self.machines.first(where: { $0.name.lowercased() == machine && $0.login == user && $0.password == password }) == nil else {
78+
return
9579
}
9680

9781
do {
98-
try self.saveToDisk(machines: machines)
82+
// Append to end of file
83+
try self.fileSystem.withLock(on: self.path, type: .exclusive) {
84+
let contents = try? self.fileSystem.readFileContents(self.path).contents
85+
try self.fileSystem.writeFileContents(self.path) { stream in
86+
// File does not exist yet
87+
if let contents = contents {
88+
stream.write(contents)
89+
stream.write("\n")
90+
}
91+
stream.write("machine \(machine) login \(user) password \(password)")
92+
stream.write("\n")
93+
}
94+
}
95+
9996
// At this point the netrc file should exist and non-empty
10097
guard let netrc = try Self.load(from: self.path) else {
10198
throw AuthorizationProviderError.other("Failed to update netrc file at \(self.path)")
10299
}
103100
self.underlying = netrc
101+
104102
callback(.success(()))
105103
} catch {
106104
callback(.failure(AuthorizationProviderError.other("Failed to update netrc file at \(self.path): \(error)")))
@@ -110,31 +108,17 @@ public struct NetrcAuthorizationProvider: AuthorizationProvider {
110108
public func authentication(for url: Foundation.URL) -> (user: String, password: String)? {
111109
self.machine(for: url).map { (user: $0.login, password: $0.password) }
112110
}
113-
114-
private func machineName(for url: Foundation.URL) -> String? {
115-
url.authenticationID
116-
}
117111

118112
private func machine(for url: Foundation.URL) -> TSCUtility.Netrc.Machine? {
119-
if let machineName = self.machineName(for: url), let machine = self.machines.first(where: { $0.name.lowercased() == machineName }) {
120-
return machine
113+
if let machine = url.authenticationID, let existing = self.machines.first(where: { $0.name.lowercased() == machine }) {
114+
return existing
121115
}
122-
if let machine = self.machines.first(where: { $0.isDefault }) {
123-
return machine
116+
if let existing = self.machines.first(where: { $0.isDefault }) {
117+
return existing
124118
}
125119
return .none
126120
}
127-
128-
private func saveToDisk(machines: [TSCUtility.Netrc.Machine]) throws {
129-
try self.fileSystem.withLock(on: self.path, type: .exclusive) {
130-
try self.fileSystem.writeFileContents(self.path) { stream in
131-
machines.forEach {
132-
stream.write("machine \($0.name) login \($0.login) password \($0.password)\n")
133-
}
134-
}
135-
}
136-
}
137-
121+
138122
private static func load(from path: AbsolutePath) throws -> TSCUtility.Netrc? {
139123
do {
140124
return try TSCUtility.Netrc.load(fromFileAtPath: path).get()
@@ -157,7 +141,7 @@ public struct KeychainAuthorizationProvider: AuthorizationProvider {
157141
public init() {}
158142

159143
public func addOrUpdate(for url: Foundation.URL, user: String, password: String, callback: @escaping (Result<Void, Error>) -> Void) {
160-
guard let server = self.server(for: url) else {
144+
guard let server = url.authenticationID else {
161145
return callback(.failure(AuthorizationProviderError.invalidURLHost))
162146
}
163147
guard let passwordData = password.data(using: .utf8) else {
@@ -176,7 +160,7 @@ public struct KeychainAuthorizationProvider: AuthorizationProvider {
176160
}
177161

178162
public func authentication(for url: Foundation.URL) -> (user: String, password: String)? {
179-
guard let server = self.server(for: url) else {
163+
guard let server = url.authenticationID else {
180164
return nil
181165
}
182166

@@ -252,10 +236,6 @@ public struct KeychainAuthorizationProvider: AuthorizationProvider {
252236
return item
253237
}
254238

255-
private func server(for url: Foundation.URL) -> String? {
256-
url.authenticationID
257-
}
258-
259239
private func `protocol`(for url: Foundation.URL) -> CFString {
260240
// See https://developer.apple.com/documentation/security/keychain_services/keychain_items/item_attribute_keys_and_values?language=swift
261241
// for a list of possible values for the `kSecAttrProtocol` attribute.

Tests/BasicsTests/AuthorizationProviderTests.swift

Lines changed: 50 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,48 +10,69 @@
1010

1111
import XCTest
1212

13-
import Basics
13+
@testable import Basics
1414
import TSCBasic
1515
import TSCTestSupport
1616

1717
final class AuthorizationProviderTests: XCTestCase {
1818
func testBasicAPIs() {
1919
struct TestProvider: AuthorizationProvider {
20-
private var map = [URL: (user: String, password: String)]()
21-
22-
mutating func addOrUpdate(for url: Foundation.URL, user: String, password: String, callback: @escaping (Result<Void, Error>) -> Void) {
23-
self.map[url] = (user, password)
24-
callback(.success(()))
25-
}
20+
let map: [URL: (user: String, password: String)]
2621

2722
func authentication(for url: URL) -> (user: String, password: String)? {
2823
return self.map[url]
2924
}
3025
}
3126

32-
var provider = TestProvider()
33-
self.run(for: &provider)
27+
let url = URL(string: "http://\(UUID().uuidString)")!
28+
let user = UUID().uuidString
29+
let password = UUID().uuidString
30+
31+
let provider = TestProvider(map: [url: (user: user, password: password)])
32+
self.assertAuthentication(provider, for: url, expected: (user, password))
3433
}
3534

3635
func testNetrc() throws {
3736
try testWithTemporaryDirectory { tmpPath in
3837
let netrcPath = tmpPath.appending(component: ".netrc")
3938

4039
var provider = try NetrcAuthorizationProvider(path: netrcPath, fileSystem: localFileSystem)
41-
self.run(for: &provider)
40+
41+
let user = UUID().uuidString
42+
43+
let url = URL(string: "http://\(UUID().uuidString)")!
44+
let password = UUID().uuidString
45+
46+
let otherURL = URL(string: "https://\(UUID().uuidString)")!
47+
let otherPassword = UUID().uuidString
48+
49+
// Add
50+
XCTAssertNoThrow(try tsc_await { callback in provider.addOrUpdate(for: url, user: user, password: password, callback: callback) })
51+
XCTAssertNoThrow(try tsc_await { callback in provider.addOrUpdate(for: otherURL, user: user, password: otherPassword, callback: callback) })
52+
53+
self.assertAuthentication(provider, for: url, expected: (user, password))
54+
55+
// Update - the new password is appended to the end of file
56+
let newPassword = UUID().uuidString
57+
XCTAssertNoThrow(try tsc_await { callback in provider.addOrUpdate(for: url, user: user, password: newPassword, callback: callback) })
58+
59+
// .netrc file now contains two entries for `url`: one with `password` and the other with `newPassword`.
60+
// `NetrcAuthorizationProvider` returns the first entry it finds.
61+
self.assertAuthentication(provider, for: url, expected: (user, password))
62+
63+
// Make sure the new entry is saved
64+
XCTAssertNotNil(provider.machines.first(where: { $0.name == url.host!.lowercased() && $0.login == user && $0.password == newPassword }))
65+
66+
self.assertAuthentication(provider, for: otherURL, expected: (user, otherPassword))
4267
}
4368
}
4469

4570
func testKeychain() throws {
4671
#if !canImport(Security) || !ENABLE_KEYCHAIN_TEST
4772
try XCTSkipIf(true)
4873
#else
49-
var provider = KeychainAuthorizationProvider()
50-
self.run(for: &provider)
51-
#endif
52-
}
53-
54-
private func run<Provider>(for provider: inout Provider) where Provider: AuthorizationProvider {
74+
let provider = KeychainAuthorizationProvider()
75+
5576
let user = UUID().uuidString
5677

5778
let url = URL(string: "http://\(UUID().uuidString)")!
@@ -63,23 +84,24 @@ final class AuthorizationProviderTests: XCTestCase {
6384
// Add
6485
XCTAssertNoThrow(try tsc_await { callback in provider.addOrUpdate(for: url, user: user, password: password, callback: callback) })
6586
XCTAssertNoThrow(try tsc_await { callback in provider.addOrUpdate(for: otherURL, user: user, password: otherPassword, callback: callback) })
66-
67-
let auth = provider.authentication(for: url)
68-
XCTAssertEqual(auth?.user, user)
69-
XCTAssertEqual(auth?.password, password)
70-
XCTAssertEqual(provider.httpAuthorizationHeader(for: url), "Basic " + "\(user):\(password)".data(using: .utf8)!.base64EncodedString())
87+
88+
self.assertAuthentication(provider, for: url, expected: (user, password))
7189

7290
// Update
7391
let newPassword = UUID().uuidString
7492
XCTAssertNoThrow(try tsc_await { callback in provider.addOrUpdate(for: url, user: user, password: newPassword, callback: callback) })
7593

76-
let updatedAuth = provider.authentication(for: url)
77-
XCTAssertEqual(updatedAuth?.user, user)
78-
XCTAssertEqual(updatedAuth?.password, newPassword)
79-
XCTAssertEqual(provider.httpAuthorizationHeader(for: url), "Basic " + "\(user):\(newPassword)".data(using: .utf8)!.base64EncodedString())
94+
// Existing password is updated
95+
self.assertAuthentication(provider, for: url, expected: (user, newPassword))
8096

81-
let otherAuth = provider.authentication(for: otherURL)
82-
XCTAssertEqual(otherAuth?.user, user)
83-
XCTAssertEqual(otherAuth?.password, otherPassword)
97+
self.assertAuthentication(provider, for: otherURL, expected: (user, otherPassword))
98+
#endif
99+
}
100+
101+
private func assertAuthentication(_ provider: AuthorizationProvider, for url: Foundation.URL, expected: (user: String, password: String)) {
102+
let authentication = provider.authentication(for: url)
103+
XCTAssertEqual(authentication?.user, expected.user)
104+
XCTAssertEqual(authentication?.password, expected.password)
105+
XCTAssertEqual(provider.httpAuthorizationHeader(for: url), "Basic " + "\(expected.user):\(expected.password)".data(using: .utf8)!.base64EncodedString())
84106
}
85107
}

0 commit comments

Comments
 (0)