Preserve port when computing the login URL (#6711)

* Preserve port when computing the login URL

* Remove extra testing target

* Add helper to compute storage key

* Add methods to add authentication to config

* Remove old test

* Update Sources/PackageRegistryTool/PackageRegistryTool+Auth.swift

Co-authored-by: Yim Lee <[email protected]>

---------

Co-authored-by: Romain Pouclet <[email protected]>
Co-authored-by: Yim Lee <[email protected]>

Author: 3 people

Sources/PackageRegistry/RegistryClient.swift

@@ -76,7 +76,7 @@ public final class RegistryClient: Cancellable {
 
         if let authorizationProvider {
             self.authorizationProvider = { url in
-                guard let registryAuthentication = configuration.authentication(for: url) else {
+                guard let registryAuthentication = try? configuration.authentication(for: url) else {
                     return .none
                 }
                 guard let (user, password) = authorizationProvider.authentication(for: url) else {

Sources/PackageRegistry/RegistryConfiguration.swift

@@ -15,6 +15,14 @@ import Foundation
 import PackageModel
 
 public struct RegistryConfiguration: Hashable {
+    static func authenticationStorageKey(for registryURL: URL) throws -> String {
+        guard let host = registryURL.host?.lowercased() else {
+            throw RegistryError.invalidURL(registryURL)
+        }
+
+        return [host, registryURL.port?.description].compactMap { $0 }.joined(separator: ":")
+    }
+
     public enum Version: Int, Codable {
         case v1 = 1
     }
@@ -66,9 +74,19 @@ public struct RegistryConfiguration: Hashable {
         self.defaultRegistry != nil || !self.scopedRegistries.isEmpty
     }
 
-    public func authentication(for registryURL: URL) -> Authentication? {
-        guard let host = registryURL.host else { return nil }
-        return self.registryAuthentication[host]
+    public func authentication(for registryURL: URL) throws -> Authentication? {
+        let key = try Self.authenticationStorageKey(for: registryURL)
+        return self.registryAuthentication[key]
+    }
+
+    public mutating func add(authentication: Authentication, for registryURL: URL) throws {
+        let key = try Self.authenticationStorageKey(for: registryURL)
+        self.registryAuthentication[key] = authentication
+    }
+
+    public mutating func removeAuthentication(for registryURL: URL) {
+        guard let key = try? Self.authenticationStorageKey(for: registryURL) else { return }
+        self.registryAuthentication.removeValue(forKey: key)
     }
 
     public func signing(for package: PackageIdentity.RegistryIdentity, registry: Registry) -> Security.Signing {

Sources/PackageRegistryTool/PackageRegistryTool+Auth.swift

@@ -90,6 +90,20 @@ private func readpassword(_ prompt: String) throws -> String {
 
 extension SwiftPackageRegistryTool {
     struct Login: SwiftCommand {
+
+        static func loginURL(from registryURL: URL, loginAPIPath: String?) throws -> URL {
+            // Login URL must be HTTPS
+            var loginURLComponents = URLComponents(url: registryURL, resolvingAgainstBaseURL: true)
+            loginURLComponents?.scheme = "https"
+            loginURLComponents?.path = loginAPIPath ?? "/login"
+
+            guard let loginURL = loginURLComponents?.url else {
+                throw ValidationError.invalidURL(registryURL)
+            }
+
+            return loginURL
+        }
+
         static let configuration = CommandConfiguration(
             abstract: "Log in to a registry"
         )
@@ -153,10 +167,6 @@ extension SwiftPackageRegistryTool {
 
             try registryURL.validateRegistryURL()
 
-            guard let host = registryURL.host?.lowercased() else {
-                throw ValidationError.invalidURL(registryURL)
-            }
-
             let authenticationType: RegistryConfiguration.AuthenticationType
             let storeUsername: String
             let storePassword: String
@@ -226,15 +236,12 @@ extension SwiftPackageRegistryTool {
                 loginAPIPath = registryURL.path
             }
 
-            // Login URL must be HTTPS
-            guard let loginURL = URL(string: "https://\(host)\(loginAPIPath ?? "/login")") else {
-                throw ValidationError.invalidURL(registryURL)
-            }
+            let loginURL = try Self.loginURL(from: registryURL, loginAPIPath: loginAPIPath)
+
 
             // Build a RegistryConfiguration with the given authentication settings
             var registryConfiguration = configuration.configuration
-            registryConfiguration
-                .registryAuthentication[host] = .init(type: authenticationType, loginAPIPath: loginAPIPath)
+            try registryConfiguration.add(authentication: .init(type: authenticationType, loginAPIPath: loginAPIPath), for: registryURL)
 
             // Build a RegistryClient to test login credentials (fingerprints don't matter in this case)
             let registryClient = RegistryClient(
@@ -307,7 +314,7 @@ extension SwiftPackageRegistryTool {
 
             // Update user-level registry configuration file
             let update: (inout RegistryConfiguration) throws -> Void = { configuration in
-                configuration.registryAuthentication[host] = .init(type: authenticationType, loginAPIPath: loginAPIPath)
+                try configuration.add(authentication: .init(type: authenticationType, loginAPIPath: loginAPIPath), for: registryURL)
             }
             try configuration.updateShared(with: update)
 
@@ -340,10 +347,6 @@ extension SwiftPackageRegistryTool {
 
             try registryURL.validateRegistryURL()
 
-            guard let host = registryURL.host?.lowercased() else {
-                throw ValidationError.invalidURL(registryURL)
-            }
-
             // We need to be able to read/write credentials
             guard let authorizationProvider = try swiftTool.getRegistryAuthorizationProvider() else {
                 throw ValidationError.unknownCredentialStore
@@ -362,7 +365,7 @@ extension SwiftPackageRegistryTool {
 
             // Update user-level registry configuration file
             let update: (inout RegistryConfiguration) throws -> Void = { configuration in
-                configuration.registryAuthentication.removeValue(forKey: host)
+                configuration.removeAuthentication(for: registryURL)
             }
             try configuration.updateShared(with: update)
 

Tests/CommandsTests/PackageRegistryToolTests.swift

@@ -903,6 +903,23 @@ final class PackageRegistryToolTests: CommandsTestCase {
         }
     }
 
+    func testCreateLoginURL() {
+        let registryURL = URL(string: "https://packages.example.com")!
+
+        XCTAssertEqual(try SwiftPackageRegistryTool.Login.loginURL(from: registryURL, loginAPIPath: nil).absoluteString, "https://packages.example.com/login")
+
+        XCTAssertEqual(try SwiftPackageRegistryTool.Login.loginURL(from: registryURL, loginAPIPath: "/secret-sign-in").absoluteString, "https://packages.example.com/secret-sign-in")
+
+    }
+
+    func testCreateLoginURLMaintainsPort() {
+        let registryURL = URL(string: "https://packages.example.com:8081")!
+
+        XCTAssertEqual(try SwiftPackageRegistryTool.Login.loginURL(from: registryURL, loginAPIPath: nil).absoluteString, "https://packages.example.com:8081/login")
+
+        XCTAssertEqual(try SwiftPackageRegistryTool.Login.loginURL(from: registryURL, loginAPIPath: "/secret-sign-in").absoluteString, "https://packages.example.com:8081/secret-sign-in")
+    }
+
     private func testRoots(callback: (Result<[[UInt8]], Error>) -> Void) {
         do {
             try fixture(name: "Signing", createGitRepo: false) { fixturePath in

Tests/PackageRegistryTests/RegistryConfigurationTests.swift

@@ -372,10 +372,10 @@ final class RegistryConfigurationTests: XCTestCase {
 
     func testGetAuthenticationConfigurationByRegistryURL() throws {
         var configuration = RegistryConfiguration()
-        configuration.registryAuthentication[defaultRegistryBaseURL.host!] = .init(type: .token)
+        try configuration.add(authentication: .init(type: .token), for: defaultRegistryBaseURL)
 
-        XCTAssertEqual(configuration.authentication(for: defaultRegistryBaseURL)?.type, .token)
-        XCTAssertNil(configuration.authentication(for: customRegistryBaseURL))
+        XCTAssertEqual(try configuration.authentication(for: defaultRegistryBaseURL)?.type, .token)
+        XCTAssertNil(try configuration.authentication(for: customRegistryBaseURL))
     }
 
     func testGetSigning_noOverrides() throws {