Skip to content

Preserve port when computing the login URL #6711

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

Merged
merged 6 commits into from
Jul 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Sources/PackageRegistry/RegistryClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
24 changes: 21 additions & 3 deletions Sources/PackageRegistry/RegistryConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down
35 changes: 19 additions & 16 deletions Sources/PackageRegistryTool/PackageRegistryTool+Auth.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand All @@ -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)

Expand Down
17 changes: 17 additions & 0 deletions Tests/CommandsTests/PackageRegistryToolTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Copy link
Contributor

Choose a reason for hiding this comment

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

<3

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
Expand Down
6 changes: 3 additions & 3 deletions Tests/PackageRegistryTests/RegistryConfigurationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down