Skip to content
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

Remove socks5 authentication check from the TransportStrategy layer #5708

Merged
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
15 changes: 5 additions & 10 deletions ios/MullvadREST/Transport/TransportStrategy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,11 @@ public class TransportStrategy: Equatable {
cipher: configuration.cipher.rawValue.description
))
case let .socks5(configuration):
switch configuration.authentication {
case .noAuthentication:
return .socks5(configuration: Socks5Configuration(proxyEndpoint: configuration.toAnyIPEndpoint))
case let .usernamePassword(username, password):
return .socks5(configuration: Socks5Configuration(
proxyEndpoint: configuration.toAnyIPEndpoint,
username: username,
password: password
))
}
return .socks5(configuration: Socks5Configuration(
proxyEndpoint: configuration.toAnyIPEndpoint,
username: configuration.credential?.username,
password: configuration.credential?.password
))
}
}

Expand Down
9 changes: 5 additions & 4 deletions ios/MullvadRESTTests/TransportStrategyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,11 @@ class TransportStrategyTests: XCTestCase {
func testUsesSocks5WithAuthenticationWhenItReaches() throws {
let username = "user"
let password = "pass"
let authentication = PersistentProxyConfiguration.SocksAuthentication.usernamePassword(
username: username,
password: password
)
let authentication = PersistentProxyConfiguration.SocksAuthentication
.authentication(PersistentProxyConfiguration.UserCredential(
username: username,
password: password
))
let socks5Configuration = PersistentProxyConfiguration.SocksConfiguration(
server: .ipv4(.loopback),
port: 1080,
Expand Down
19 changes: 18 additions & 1 deletion ios/MullvadSettings/PersistentAccessMethod.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,17 @@ extension PersistentProxyConfiguration {
/// Socks autentication method.
public enum SocksAuthentication: Codable {
case noAuthentication
case usernamePassword(username: String, password: String)
case authentication(UserCredential)
}

public struct UserCredential: Codable {
public let username: String
public let password: String

public init(username: String, password: String) {
self.username = username
self.password = password
}
}

/// Socks v5 proxy configuration.
Expand All @@ -75,6 +85,13 @@ extension PersistentProxyConfiguration {
self.authentication = authentication
}

public var credential: UserCredential? {
guard case let .authentication(credential) = authentication else {
return nil
}
return credential
}

public var toAnyIPEndpoint: AnyIPEndpoint {
switch server {
case let .ipv4(ip):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@ extension AccessMethodViewModel.Socks {
context: context
))
} else {
draftConfiguration.authentication = .usernamePassword(username: username, password: password)
draftConfiguration.authentication = .authentication(PersistentProxyConfiguration.UserCredential(
username: username,
password: password
))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ extension PersistentProxyConfiguration {
)

switch config.authentication {
case let .usernamePassword(username, password):
socks.username = username
socks.password = password
case let .authentication(userCredential):
socks.username = userCredential.username
socks.password = userCredential.password
socks.authenticate = true

case .noAuthentication:
Expand Down
Loading