Allow editing of OpenVPN endpoints (#335)

Hosts only:

- Add new
- Edit/delete existing
- Reorder

Closes #206

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Added
+
+- OpenVPN: Allow editing of endpoints. [#335](https://github.com/passepartoutvpn/passepartout-apple/pull/335)
+
 ### Changed
 
 - OpenVPN: Endpoint UX. [#332](https://github.com/passepartoutvpn/passepartout-apple/pull/332)

Passepartout.xcodeproj/project.pbxproj

@@ -202,6 +202,7 @@
 		A3A7CC4A28790BD900172D7D /* Theme.swift in Sources */ = {isa = PBXBuildFile; fileRef = A3A7CC4928790BD900172D7D /* Theme.swift */; };
 		A3A7CC56287D56E800172D7D /* ProviderLocationItem.swift in Sources */ = {isa = PBXBuildFile; fileRef = A3A7CC55287D56E800172D7D /* ProviderLocationItem.swift */; };
 		A3A7CC58287D576400172D7D /* ProviderLocationItem+ViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = A3A7CC57287D576400172D7D /* ProviderLocationItem+ViewModel.swift */; };
+		A3D5B04C2A6C6CF2008016D5 /* EndpointView+Add.swift in Sources */ = {isa = PBXBuildFile; fileRef = A3D5B04B2A6C6CF2008016D5 /* EndpointView+Add.swift */; };
 /* End PBXBuildFile section */
 
 /* Begin PBXContainerItemProxy section */
@@ -523,6 +524,7 @@
 		A3A7CC4928790BD900172D7D /* Theme.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Theme.swift; sourceTree = "<group>"; };
 		A3A7CC55287D56E800172D7D /* ProviderLocationItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProviderLocationItem.swift; sourceTree = "<group>"; };
 		A3A7CC57287D576400172D7D /* ProviderLocationItem+ViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ProviderLocationItem+ViewModel.swift"; sourceTree = "<group>"; };
+		A3D5B04B2A6C6CF2008016D5 /* EndpointView+Add.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "EndpointView+Add.swift"; sourceTree = "<group>"; };
 /* End PBXFileReference section */
 
 /* Begin PBXFrameworksBuildPhase section */
@@ -651,6 +653,7 @@
 				0E49F6BA27D7638300385834 /* EndpointAdvancedView+OpenVPN.swift */,
 				0E49F6BC27D7639000385834 /* EndpointAdvancedView+WireGuard.swift */,
 				0E71ACEA27C1060D00F85C4B /* EndpointView.swift */,
+				A3D5B04B2A6C6CF2008016D5 /* EndpointView+Add.swift */,
 				0E5349C527C176C200C71BB3 /* EndpointView+OpenVPN.swift */,
 				0E5349C727C176D100C71BB3 /* EndpointView+WireGuard.swift */,
 				0EB90CC029C25BBD00E64628 /* InteractiveConnectionView.swift */,
@@ -1458,6 +1461,7 @@
 				0EBC075D27EC529000208AD9 /* DebugLog+Constants.swift in Sources */,
 				0E3CD47F280DA14B007075C0 /* AddProfileMenu.swift in Sources */,
 				0EB17EAA27D226C900D473B5 /* Constants+App.swift in Sources */,
+				A3D5B04C2A6C6CF2008016D5 /* EndpointView+Add.swift in Sources */,
 				0E3B7FD627E5173A00C66F13 /* ProfileView+VPN.swift in Sources */,
 				0ED89C1E27DE3F8D008B36D6 /* IntentAddView.swift in Sources */,
 				0E5468002867AC9A00F74D1C /* MacUtils.swift in Sources */,

Passepartout/App/Constants/Theme.swift

@@ -348,6 +348,10 @@ extension View {
         tint(themePrimaryBackgroundColor)
     }
 
+    func themeDestructiveTintStyle() -> some View {
+        tint(themeErrorColor)
+    }
+
     func themeTextButtonStyle() -> some View {
         accentColor(.primary)
     }
@@ -536,8 +540,9 @@ extension View {
             .themeRawTextStyle()
     }
 
-    func themeValidSocketPort() -> some View {
-        keyboardType(.numberPad)
+    func themeValidSocketPort(_ port: String?) -> some View {
+        themeValidating(port, validator: Validators.socketPort)
+            .keyboardType(.numberPad)
     }
 
     func themeValidDomainName(_ domainName: String?) -> some View {

Passepartout/App/L10n/TunnelKit+L10n.swift

@@ -110,12 +110,12 @@ extension IPv6Settings: StyledLocalizableEntity {
 
 extension IPv4Settings.Route: LocalizableEntity {
     public var localizedDescription: String {
-        "\(destination)/\(mask) -> \(gateway ?? "*")"
+        "\(destination)/\(mask) → \(gateway ?? "*")"
     }
 }
 
 extension IPv6Settings.Route: LocalizableEntity {
     public var localizedDescription: String {
-        "\(destination)/\(prefixLength) -> \(gateway ?? "*")"
+        "\(destination)/\(prefixLength) → \(gateway ?? "*")"
     }
 }

Passepartout/App/Views/EndpointView+Add.swift

@@ -0,0 +1,113 @@
+//
+//  EndpointView+Add.swift
+//  Passepartout
+//
+//  Created by Davide De Rosa on 7/22/23.
+//  Copyright (c) 2023 Davide De Rosa. All rights reserved.
+//
+//  https://github.com/passepartoutvpn
+//
+//  This file is part of Passepartout.
+//
+//  Passepartout is free software: you can redistribute it and/or modify
+//  it under the terms of the GNU General Public License as published by
+//  the Free Software Foundation, either version 3 of the License, or
+//  (at your option) any later version.
+//
+//  Passepartout is distributed in the hope that it will be useful,
+//  but WITHOUT ANY WARRANTY; without even the implied warranty of
+//  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+//  GNU General Public License for more details.
+//
+//  You should have received a copy of the GNU General Public License
+//  along with Passepartout.  If not, see <http://www.gnu.org/licenses/>.
+//
+
+import PassepartoutLibrary
+import SwiftUI
+import TunnelKitCore
+
+extension EndpointView {
+    struct AddView: View {
+        @Environment(\.presentationMode) private var presentationMode
+
+        private let title: String
+
+        private let endpoint: Endpoint?
+
+        private let onSave: ((Endpoint, Endpoint?) -> Void)?
+
+        @State private var socketType: SocketType = .udp
+
+        @State private var address = ""
+
+        @State private var port = ""
+
+        @State private var didAppear = false
+
+        private let allSocketTypes: [SocketType] = [
+            .udp,
+            .udp4,
+            .udp6,
+            .tcp,
+            .tcp4,
+            .tcp6
+        ]
+
+        init(_ title: String, endpoint: Endpoint? = nil, onSave: ((Endpoint, Endpoint?) -> Void)? = nil) {
+            self.title = title
+            self.endpoint = endpoint
+            self.onSave = onSave
+        }
+
+        var body: some View {
+            List {
+                Section {
+                    themeTextPicker(
+                        L10n.Global.Strings.protocol,
+                        selection: $socketType,
+                        values: allSocketTypes,
+                        description: \.rawValue
+                    )
+                    TextField(L10n.Global.Strings.address, text: $address, onCommit: commitChanges)
+                        .themeValidIPAddress(address)
+                    TextField(L10n.Global.Strings.port, text: $port, onCommit: commitChanges)
+                        .themeValidSocketPort(port)
+                }
+            }.onAppear {
+                guard !didAppear, let endpoint else {
+                    return
+                }
+                socketType = endpoint.proto.socketType
+                address = endpoint.address
+                port = String(endpoint.proto.port)
+                didAppear = true
+            }.themeSecondaryView()
+            .navigationTitle(title)
+            .toolbar {
+                themeCloseItem(presentationMode: presentationMode)
+                ToolbarItem(placement: .primaryAction) {
+                    Button(action: commitChanges, label: themeSaveButtonLabel)
+                }
+            }
+        }
+    }
+}
+
+// MARK: -
+
+private extension EndpointView.AddView {
+}
+
+// MARK: -
+
+private extension EndpointView.AddView {
+    func commitChanges() {
+        let endpointString = "\(address):\(socketType.rawValue):\(port)"
+        guard let newEndpoint = Endpoint(rawValue: endpointString) else {
+            return
+        }
+        onSave?(newEndpoint, endpoint)
+        presentationMode.wrappedValue.dismiss()
+    }
+}

Passepartout/App/Views/EndpointView+OpenVPN.swift

@@ -41,6 +41,10 @@ extension EndpointView {
 
         @State private var isExpanded: [String: Bool] = [:]
 
+        @State private var isAdding = false
+
+        @State private var editedEndpoint: Endpoint?
+
         init(currentProfile: ObservableProfile) {
             let providerManager: ProviderManager = .shared
 
@@ -54,7 +58,11 @@ extension EndpointView {
             ScrollViewReader { scrollProxy in
                 List {
                     mainSection
-                    endpointsSections
+                    if isConfigurationReadonly {
+                        groupedEndpointsSections
+                    } else {
+                        endpointsSection
+                    }
                     advancedSection
                 }.onAppear {
                     isAutomatic = (currentProfile.value.customEndpoint == nil)
@@ -63,6 +71,11 @@ extension EndpointView {
                     }
                     scrollToCustomEndpoint(scrollProxy)
                 }.onChange(of: isAutomatic, perform: onToggleAutomatic)
+                .toolbar {
+                    if !isConfigurationReadonly {
+                        addButton
+                    }
+                }
             }.navigationTitle(L10n.Global.Strings.endpoint)
         }
     }
@@ -71,44 +84,19 @@ extension EndpointView {
 // MARK: -
 
 private extension EndpointView.OpenVPNView {
+    var isConfigurationReadonly: Bool {
+        currentProfile.value.isProvider
+    }
+
     var mainSection: some View {
         Section {
             Toggle(L10n.Global.Strings.automatic, isOn: $isAutomatic.themeAnimation())
         } footer: {
-            // FIXME: l10n
+            // FIXME: l10n, endpoint
             themeErrorMessage(isManualEndpointRequired ? L10n.Endpoint.Errors.endpointRequired : nil)
         }
     }
 
-    var endpointsSections: some View {
-        ForEach(endpointsByAddress, content: endpointsGroup(forSection:))
-            .disabled(isAutomatic)
-    }
-
-    // TODO: OpenVPN, make endpoints editable
-    func endpointsGroup(forSection section: EndpointsByAddress) -> some View {
-        Section {
-            DisclosureGroup(isExpanded: isExpandedBinding(address: section.address)) {
-                ForEach(section.endpoints) {
-                    row(forEndpoint: $0)
-                }
-            } label: {
-                Text(L10n.Global.Strings.address)
-                    .withTrailingText(section.address)
-            }
-        }
-    }
-
-    func row(forEndpoint endpoint: Endpoint) -> some View {
-        Button {
-            withAnimation {
-                currentProfile.value.customEndpoint = endpoint
-            }
-        } label: {
-            Text(endpoint.proto.rawValue)
-        }.withTrailingCheckmark(when: currentProfile.value.customEndpoint == endpoint)
-    }
-
     var advancedSection: some View {
         Section {
             let caption = L10n.Endpoint.Advanced.title
@@ -122,47 +110,14 @@ private extension EndpointView.OpenVPNView {
         }
     }
 
-    var endpointsByAddress: [EndpointsByAddress] {
-        guard let remotes = builder.remotes, !remotes.isEmpty else {
-            return []
-        }
-        var uniqueAddresses: [String] = []
-        remotes.forEach {
-            guard !uniqueAddresses.contains($0.address) else {
-                return
-            }
-            uniqueAddresses.append($0.address)
-        }
-        return uniqueAddresses.map {
-            EndpointsByAddress(address: $0, remotes: remotes)
-        }
-    }
-
     var isManualEndpointRequired: Bool {
         !isAutomatic && currentProfile.value.customEndpoint == nil
     }
-
-    var isConfigurationReadonly: Bool {
-        currentProfile.value.isProvider
-    }
 }
 
-private struct EndpointsByAddress: Identifiable {
-    let address: String
-
-    let endpoints: [Endpoint]
-
-    init(address: String, remotes: [Endpoint]?) {
-        self.address = address
-        endpoints = remotes?.filter {
-            $0.address == address
-        }.sorted() ?? []
-    }
-
-    // MARK: Identifiable
-
-    var id: String {
-        address
+private extension Endpoint {
+    var linearDescription: String {
+        "\(address) → \(proto.rawValue)"
     }
 }
 
@@ -187,6 +142,180 @@ private extension EndpointView.OpenVPNView {
     }
 }
 
+// MARK: - Editable: linear
+
+private extension EndpointView.OpenVPNView {
+    var endpointsSection: some View {
+        Section {
+            ForEach(builder.remotes ?? []) { endpoint in
+                fullRowForEndpoint(endpoint)
+                    .swipeActions(edge: .trailing, allowsFullSwipe: false) {
+                        actions(forEndpoint: endpoint)
+                    }
+            }.onMove(perform: moveEndpoints)
+                .disabled(isAutomatic)
+        }
+    }
+
+    func fullRowForEndpoint(_ endpoint: Endpoint) -> some View {
+        Button {
+            withAnimation {
+                currentProfile.value.customEndpoint = endpoint
+            }
+        } label: {
+            Text(endpoint.linearDescription)
+        }.sheet(item: $editedEndpoint) { endpoint in
+            NavigationView {
+                EndpointView.AddView(L10n.Global.Strings.edit, endpoint: endpoint, onSave: commitEndpoint)
+            }.themeGlobal()
+        }.withTrailingCheckmark(when: currentProfile.value.customEndpoint == endpoint)
+    }
+
+    @ViewBuilder
+    func actions(forEndpoint endpoint: Endpoint) -> some View {
+        if !isConfigurationReadonly {
+            if (builder.remotes?.count ?? 0) > 1 {
+                removeButton(forEndpoint: endpoint)
+            }
+            editButton(forEndpoint: endpoint)
+        }
+    }
+
+    var addButton: some View {
+        Button {
+            isAdding = true
+        } label: {
+            themeAddMenuImage.asSystemImage
+        }.sheet(isPresented: $isAdding) {
+            NavigationView {
+                EndpointView.AddView(L10n.Global.Strings.add, onSave: commitEndpoint)
+            }.themeGlobal()
+        }
+    }
+
+    func removeButton(forEndpoint endpoint: Endpoint) -> some View {
+        Button(role: .destructive) {
+            deleteEndpoint(endpoint)
+        } label: {
+            Text(L10n.Global.Strings.delete)
+        }.themeDestructiveTintStyle()
+    }
+
+    func editButton(forEndpoint endpoint: Endpoint) -> some View {
+        Button {
+            editedEndpoint = endpoint
+        } label: {
+            Text(L10n.Global.Strings.edit)
+        }.themePrimaryTintStyle()
+    }
+}
+
+private extension EndpointView.OpenVPNView {
+    func commitEndpoint(_ newEndpoint: Endpoint, editedEndpoint: Endpoint?) {
+        withAnimation {
+
+            // replace existing
+            if let editedEndpoint,
+               let editedIndex = builder.remotes?.firstIndex(where: { $0 == editedEndpoint }) {
+
+                builder.remotes?[editedIndex] = newEndpoint
+                if currentProfile.value.customEndpoint == editedEndpoint {
+                    currentProfile.value.customEndpoint = newEndpoint
+                }
+            }
+            // add new
+            else {
+                if builder.remotes != nil {
+                    builder.remotes?.append(newEndpoint)
+                } else {
+                    assertionFailure("Nil remotes, how did we get here?")
+                    builder.remotes = [newEndpoint]
+                }
+            }
+        }
+    }
+
+    func moveEndpoints(fromOffsets: IndexSet, toOffset: Int) {
+        builder.remotes?.move(fromOffsets: fromOffsets, toOffset: toOffset)
+    }
+
+    func deleteEndpoint(_ endpoint: Endpoint) {
+        withAnimation {
+            builder.remotes?.removeAll {
+                $0 == endpoint
+            }
+            if currentProfile.value.customEndpoint == endpoint {
+                currentProfile.value.customEndpoint = nil
+            }
+        }
+    }
+}
+
+// MARK: - Non-editable: group by address
+
+private extension EndpointView.OpenVPNView {
+    var groupedEndpointsSections: some View {
+        ForEach(endpointsByAddress, content: endpointsGroup(forSection:))
+            .disabled(isAutomatic)
+    }
+
+    func endpointsGroup(forSection section: EndpointsByAddress) -> some View {
+        Section {
+            DisclosureGroup(isExpanded: isExpandedBinding(address: section.address)) {
+                ForEach(section.endpoints, content: groupedRowForEndpoint)
+            } label: {
+                Text(L10n.Global.Strings.address)
+                    .withTrailingText(section.address)
+            }
+        }
+    }
+
+    func groupedRowForEndpoint(_ endpoint: Endpoint) -> some View {
+        Button {
+            withAnimation {
+                currentProfile.value.customEndpoint = endpoint
+            }
+        } label: {
+            Text(endpoint.proto.rawValue)
+        }.withTrailingCheckmark(when: currentProfile.value.customEndpoint == endpoint)
+    }
+
+    var endpointsByAddress: [EndpointsByAddress] {
+        guard let remotes = builder.remotes, !remotes.isEmpty else {
+            return []
+        }
+        var uniqueAddresses: [String] = []
+        remotes.forEach {
+            guard !uniqueAddresses.contains($0.address) else {
+                return
+            }
+            uniqueAddresses.append($0.address)
+        }
+        return uniqueAddresses.map {
+            EndpointsByAddress(address: $0, remotes: remotes)
+        }
+    }
+}
+
+private struct EndpointsByAddress: Identifiable {
+    let address: String
+
+    let endpoints: [Endpoint]
+
+    init(address: String, remotes: [Endpoint]?) {
+        self.address = address
+        endpoints = remotes?.filter {
+            $0.address == address
+        }.sorted() ?? []
+    }
+
+    // MARK: Identifiable
+
+    var id: String {
+        address
+    }
+}
+
 // MARK: - Bindings
 
 private extension ObservableProfile {

Passepartout/App/Views/NetworkSettingsView.swift

@@ -204,7 +204,7 @@ private extension NetworkSettingsView {
                         .withLeadingText(L10n.Global.Strings.address)
 
                     TextField(Unlocalized.Placeholders.port, text: $settings.proxy.proxyPort.toString())
-                        .themeValidSocketPort()
+                        .themeValidSocketPort(settings.proxy.proxyPort?.description)
                         .withLeadingText(L10n.Global.Strings.port)
 
                 case .pac:

Passepartout/App/en.lproj/Localizable.strings

@@ -53,6 +53,7 @@
 "global.strings.authentication" = "Authentication";
 "global.strings.policy" = "Policy";
 "global.strings.networks" = "Networks";
+"global.strings.edit" = "Edit";
 "global.messages.unlock_app" = "Passepartout is locked";
 "global.messages.email_not_configured" = "No e-mail account is configured.";
 "global.messages.share" = "Passepartout is a user-friendly, open source OpenVPN / WireGuard client for iOS and macOS";

Passepartout/AppShared/Constants/SwiftGen+Strings.swift

@@ -492,6 +492,8 @@ internal enum L10n {
       internal static let download = L10n.tr("Localizable", "global.strings.download", fallback: "Download")
       /// Duplicate
       internal static let duplicate = L10n.tr("Localizable", "global.strings.duplicate", fallback: "Duplicate")
+      /// Edit
+      internal static let edit = L10n.tr("Localizable", "global.strings.edit", fallback: "Edit")
       /// Enabled
       internal static let enabled = L10n.tr("Localizable", "global.strings.enabled", fallback: "Enabled")
       /// Encryption

PassepartoutLibrary/Sources/PassepartoutCore/Reusable/Validators.swift

@@ -33,6 +33,8 @@ public struct Validators {
 
         case ipAddress
 
+        case socketPort
+
         case domainName
 
         case wildcardDomainName
@@ -68,6 +70,13 @@ public struct Validators {
         }
     }
 
+    public static func socketPort(_ string: String) throws {
+        guard let num = Int(string),
+              (Int(UInt16.min)...Int(UInt16.max)).contains(num) else {
+            throw ValidationError.socketPort
+        }
+    }
+
     public static func domainName(_ string: String) throws {
         guard rxDomainName.numberOfMatches(in: string, options: [], range: .init(location: 0, length: string.count)) > 0 else {
             throw ValidationError.domainName