Skip to content

refactor: merge session & settings abstractions #46

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 1 commit into from
Feb 19, 2025
Merged
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
20 changes: 8 additions & 12 deletions Coder Desktop/Coder Desktop/Coder_DesktopApp.swift
Original file line number Diff line number Diff line change
@@ -11,15 +11,14 @@ struct DesktopApp: App {
EmptyView()
}
Window("Sign In", id: Windows.login.rawValue) {
LoginForm<SecureSession>()
.environmentObject(appDelegate.session)
.environmentObject(appDelegate.settings)
LoginForm()
.environmentObject(appDelegate.state)
}
.windowResizability(.contentSize)
SwiftUI.Settings {
SettingsView<CoderVPNService>()
.environmentObject(appDelegate.vpn)
.environmentObject(appDelegate.settings)
.environmentObject(appDelegate.state)
}
.windowResizability(.contentSize)
}
@@ -29,28 +28,25 @@ struct DesktopApp: App {
class AppDelegate: NSObject, NSApplicationDelegate {
private var menuBarExtra: FluidMenuBarExtra?
let vpn: CoderVPNService
let session: SecureSession
let settings: Settings
let state: AppState

override init() {
vpn = CoderVPNService()
settings = Settings()
session = SecureSession(onChange: vpn.configureTunnelProviderProtocol)
state = AppState(onChange: vpn.configureTunnelProviderProtocol)
}

func applicationDidFinishLaunching(_: Notification) {
menuBarExtra = FluidMenuBarExtra(title: "Coder Desktop", image: "MenuBarIcon") {
VPNMenu<CoderVPNService, SecureSession>().frame(width: 256)
VPNMenu<CoderVPNService>().frame(width: 256)
.environmentObject(self.vpn)
.environmentObject(self.session)
.environmentObject(self.settings)
.environmentObject(self.state)
}
}

// This function MUST eventually call `NSApp.reply(toApplicationShouldTerminate: true)`
// or return `.terminateNow`
func applicationShouldTerminate(_: NSApplication) -> NSApplication.TerminateReply {
if !settings.stopVPNOnQuit { return .terminateNow }
if !state.stopVPNOnQuit { return .terminateNow }
Task {
await vpn.stop()
NSApp.reply(toApplicationShouldTerminate: true)
29 changes: 0 additions & 29 deletions Coder Desktop/Coder Desktop/Preview Content/PreviewSession.swift

This file was deleted.

84 changes: 43 additions & 41 deletions Coder Desktop/Coder Desktop/State.swift
Original file line number Diff line number Diff line change
@@ -4,28 +4,20 @@ import KeychainAccess
import NetworkExtension
import SwiftUI

protocol Session: ObservableObject {
var hasSession: Bool { get }
var baseAccessURL: URL? { get }
var sessionToken: String? { get }

func store(baseAccessURL: URL, sessionToken: String)
func clear()
func tunnelProviderProtocol() -> NETunnelProviderProtocol?
}

class SecureSession: ObservableObject, Session {
class AppState: ObservableObject {
let appId = Bundle.main.bundleIdentifier!

// Stored in UserDefaults
@Published private(set) var hasSession: Bool {
didSet {
guard persistent else { return }
UserDefaults.standard.set(hasSession, forKey: Keys.hasSession)
}
}

@Published private(set) var baseAccessURL: URL? {
didSet {
guard persistent else { return }
UserDefaults.standard.set(baseAccessURL, forKey: Keys.baseAccessURL)
}
}
@@ -37,6 +29,27 @@ class SecureSession: ObservableObject, Session {
}
}

@Published var useLiteralHeaders: Bool = UserDefaults.standard.bool(forKey: Keys.useLiteralHeaders) {
didSet {
guard persistent else { return }
UserDefaults.standard.set(useLiteralHeaders, forKey: Keys.useLiteralHeaders)
}
}

@Published var literalHeaders: [LiteralHeader] {
didSet {
guard persistent else { return }
try? UserDefaults.standard.set(JSONEncoder().encode(literalHeaders), forKey: Keys.literalHeaders)
}
}

@Published var stopVPNOnQuit: Bool = UserDefaults.standard.bool(forKey: Keys.stopVPNOnQuit) {
didSet {
guard persistent else { return }
UserDefaults.standard.set(stopVPNOnQuit, forKey: Keys.stopVPNOnQuit)
}
}

func tunnelProviderProtocol() -> NETunnelProviderProtocol? {
if !hasSession { return nil }
let proto = NETunnelProviderProtocol()
@@ -49,37 +62,50 @@ class SecureSession: ObservableObject, Session {
}

private let keychain: Keychain
private let persistent: Bool

let onChange: ((NETunnelProviderProtocol?) -> Void)?

public init(onChange: ((NETunnelProviderProtocol?) -> Void)? = nil) {
public init(onChange: ((NETunnelProviderProtocol?) -> Void)? = nil,
persistent: Bool = true)
{
self.persistent = persistent
self.onChange = onChange
keychain = Keychain(service: Bundle.main.bundleIdentifier!)
_hasSession = Published(initialValue: UserDefaults.standard.bool(forKey: Keys.hasSession))
_baseAccessURL = Published(initialValue: UserDefaults.standard.url(forKey: Keys.baseAccessURL))
_hasSession = Published(initialValue: persistent ? UserDefaults.standard.bool(forKey: Keys.hasSession) : false)
_baseAccessURL = Published(
initialValue: persistent ? UserDefaults.standard.url(forKey: Keys.baseAccessURL) : nil
)
_literalHeaders = Published(
initialValue: persistent ? UserDefaults.standard.data(
forKey: Keys.literalHeaders
).flatMap { try? JSONDecoder().decode([LiteralHeader].self, from: $0) } ?? [] : []
)
if hasSession {
_sessionToken = Published(initialValue: keychainGet(for: Keys.sessionToken))
}
}

public func store(baseAccessURL: URL, sessionToken: String) {
public func login(baseAccessURL: URL, sessionToken: String) {
hasSession = true
self.baseAccessURL = baseAccessURL
self.sessionToken = sessionToken
if let onChange { onChange(tunnelProviderProtocol()) }
}

public func clear() {
public func clearSession() {
hasSession = false
sessionToken = nil
if let onChange { onChange(tunnelProviderProtocol()) }
}

private func keychainGet(for key: String) -> String? {
try? keychain.getString(key)
guard persistent else { return nil }
return try? keychain.getString(key)
}

private func keychainSet(_ value: String?, for key: String) {
guard persistent else { return }
if let value {
try? keychain.set(value, key: key)
} else {
@@ -91,31 +117,7 @@ class SecureSession: ObservableObject, Session {
static let hasSession = "hasSession"
static let baseAccessURL = "baseAccessURL"
static let sessionToken = "sessionToken"
}
}

class Settings: ObservableObject {
private let store: UserDefaults
@AppStorage(Keys.useLiteralHeaders) var useLiteralHeaders = false

@Published var literalHeaders: [LiteralHeader] {
didSet {
try? store.set(JSONEncoder().encode(literalHeaders), forKey: Keys.literalHeaders)
}
}

@AppStorage(Keys.stopVPNOnQuit) var stopVPNOnQuit = true

init(store: UserDefaults = .standard) {
self.store = store
_literalHeaders = Published(
initialValue: store.data(
forKey: Keys.literalHeaders
).flatMap { try? JSONDecoder().decode([LiteralHeader].self, from: $0) } ?? []
)
}

enum Keys {
static let useLiteralHeaders = "UseLiteralHeaders"
static let literalHeaders = "LiteralHeaders"
static let stopVPNOnQuit = "StopVPNOnQuit"
6 changes: 3 additions & 3 deletions Coder Desktop/Coder Desktop/Views/Agents.swift
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import SwiftUI

struct Agents<VPN: VPNService, S: Session>: View {
struct Agents<VPN: VPNService>: View {
@EnvironmentObject var vpn: VPN
@EnvironmentObject var session: S
@EnvironmentObject var state: AppState
@State private var viewAll = false
private let defaultVisibleRows = 5

@@ -15,7 +15,7 @@ struct Agents<VPN: VPNService, S: Session>: View {
let items = vpn.menuState.sorted
let visibleItems = viewAll ? items[...] : items.prefix(defaultVisibleRows)
ForEach(visibleItems, id: \.id) { agent in
MenuItemView(item: agent, baseAccessURL: session.baseAccessURL!)
MenuItemView(item: agent, baseAccessURL: state.baseAccessURL!)
.padding(.horizontal, Theme.Size.trayMargin)
}
if items.count == 0 {
10 changes: 5 additions & 5 deletions Coder Desktop/Coder Desktop/Views/AuthButton.swift
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
import SwiftUI

struct AuthButton<VPN: VPNService, S: Session>: View {
@EnvironmentObject var session: S
struct AuthButton<VPN: VPNService>: View {
@EnvironmentObject var state: AppState
@EnvironmentObject var vpn: VPN
@Environment(\.openWindow) var openWindow

var body: some View {
Button {
if session.hasSession {
if state.hasSession {
Task {
await vpn.stop()
session.clear()
state.clearSession()
}
} else {
openWindow(id: .login)
}
} label: {
ButtonRowView {
Text(session.hasSession ? "Sign out" : "Sign in")
Text(state.hasSession ? "Sign out" : "Sign in")
}
}.buttonStyle(.plain)
}
15 changes: 7 additions & 8 deletions Coder Desktop/Coder Desktop/Views/LoginForm.swift
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import CoderSDK
import SwiftUI

struct LoginForm<S: Session>: View {
@EnvironmentObject var session: S
@EnvironmentObject var settings: Settings
struct LoginForm: View {
@EnvironmentObject var state: AppState
@Environment(\.dismiss) private var dismiss

@State private var baseAccessURL: String = ""
@@ -38,7 +37,7 @@ struct LoginForm<S: Session>: View {
}
.animation(.easeInOut, value: currentPage)
.onAppear {
baseAccessURL = session.baseAccessURL?.absoluteString ?? baseAccessURL
baseAccessURL = state.baseAccessURL?.absoluteString ?? baseAccessURL
sessionToken = ""
}
.alert("Error", isPresented: Binding(
@@ -72,14 +71,14 @@ struct LoginForm<S: Session>: View {
}
loading = true
defer { loading = false }
let client = Client(url: url, token: sessionToken, headers: settings.literalHeaders.map { $0.toSDKHeader() })
let client = Client(url: url, token: sessionToken, headers: state.literalHeaders.map { $0.toSDKHeader() })
do {
_ = try await client.user("me")
} catch {
loginError = .failedAuth(error)
return
}
session.store(baseAccessURL: url, sessionToken: sessionToken)
state.login(baseAccessURL: url, sessionToken: sessionToken)
dismiss()
}

@@ -219,7 +218,7 @@ enum LoginField: Hashable {

#if DEBUG
#Preview {
LoginForm<PreviewSession>()
.environmentObject(PreviewSession())
LoginForm()
.environmentObject(AppState())
}
#endif
4 changes: 2 additions & 2 deletions Coder Desktop/Coder Desktop/Views/Settings/GeneralTab.swift
Original file line number Diff line number Diff line change
@@ -2,14 +2,14 @@ import LaunchAtLogin
import SwiftUI

struct GeneralTab: View {
@EnvironmentObject var settings: Settings
@EnvironmentObject var state: AppState
var body: some View {
Form {
Section {
LaunchAtLogin.Toggle("Launch at Login")
}
Section {
Toggle(isOn: $settings.stopVPNOnQuit) {
Toggle(isOn: $state.stopVPNOnQuit) {
Text("Stop VPN on Quit")
}
}
Original file line number Diff line number Diff line change
@@ -3,7 +3,7 @@ import SwiftUI
struct LiteralHeaderModal: View {
var existingHeader: LiteralHeader?

@EnvironmentObject var settings: Settings
@EnvironmentObject var state: AppState
@Environment(\.dismiss) private var dismiss

@State private var header: String = ""
@@ -35,11 +35,11 @@ struct LiteralHeaderModal: View {
func submit() {
defer { dismiss() }
if let existingHeader {
settings.literalHeaders.removeAll { $0 == existingHeader }
state.literalHeaders.removeAll { $0 == existingHeader }
}
let newHeader = LiteralHeader(header: header, value: value)
if !settings.literalHeaders.contains(newHeader) {
settings.literalHeaders.append(newHeader)
if !state.literalHeaders.contains(newHeader) {
state.literalHeaders.append(newHeader)
}
}
}
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ import SwiftUI

struct LiteralHeadersSection<VPN: VPNService>: View {
@EnvironmentObject var vpn: VPN
@EnvironmentObject var settings: Settings
@EnvironmentObject var state: AppState

@State private var selectedHeader: LiteralHeader.ID?
@State private var editingHeader: LiteralHeader?
@@ -12,17 +12,17 @@ struct LiteralHeadersSection<VPN: VPNService>: View {

var body: some View {
Section {
Toggle(isOn: settings.$useLiteralHeaders) {
Toggle(isOn: $state.useLiteralHeaders) {
Text("HTTP Headers")
Text("When enabled, these headers will be included on all outgoing HTTP requests.")
if vpn.state != .disabled { Text("Cannot be modified while Coder VPN is enabled.") }
}
.controlSize(.large)

Table(settings.literalHeaders, selection: $selectedHeader) {
Table(state.literalHeaders, selection: $selectedHeader) {
TableColumn("Header", value: \.header)
TableColumn("Value", value: \.value)
}.opacity(settings.useLiteralHeaders ? 1 : 0.5)
}.opacity(state.useLiteralHeaders ? 1 : 0.5)
.frame(minWidth: 400, minHeight: 200)
.padding(.bottom, 25)
.overlay(alignment: .bottom) {
@@ -37,7 +37,7 @@ struct LiteralHeadersSection<VPN: VPNService>: View {
}
Divider()
Button {
settings.literalHeaders.removeAll { $0.id == selectedHeader }
state.literalHeaders.removeAll { $0.id == selectedHeader }
selectedHeader = nil
} label: {
Image(systemName: "minus")
@@ -53,10 +53,10 @@ struct LiteralHeadersSection<VPN: VPNService>: View {
.contextMenu(forSelectionType: LiteralHeader.ID.self, menu: { _ in },
primaryAction: { selectedHeaders in
if let firstHeader = selectedHeaders.first {
editingHeader = settings.literalHeaders.first(where: { $0.id == firstHeader })
editingHeader = state.literalHeaders.first(where: { $0.id == firstHeader })
}
})
.disabled(!settings.useLiteralHeaders)
.disabled(!state.useLiteralHeaders)
}
.sheet(isPresented: $addingNewHeader) {
LiteralHeaderModal()
22 changes: 11 additions & 11 deletions Coder Desktop/Coder Desktop/Views/VPNMenu.swift
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import SwiftUI

struct VPNMenu<VPN: VPNService, S: Session>: View {
struct VPNMenu<VPN: VPNService>: View {
@EnvironmentObject var vpn: VPN
@EnvironmentObject var session: S
@EnvironmentObject var state: AppState
@Environment(\.openSettings) private var openSettings

// There appears to be a race between the VPN service reporting itself as disconnected,
@@ -38,17 +38,17 @@ struct VPNMenu<VPN: VPNService, S: Session>: View {
Text("Workspaces")
.font(.headline)
.foregroundColor(.gray)
VPNState<VPN, S>()
VPNState<VPN>()
}.padding([.horizontal, .top], Theme.Size.trayInset)
Agents<VPN, S>()
Agents<VPN>()
// Trailing stack
VStack(alignment: .leading, spacing: 3) {
TrayDivider()
if vpn.state == .connected, !vpn.menuState.invalidAgents.isEmpty {
InvalidAgentsButton<VPN>()
}
if session.hasSession {
Link(destination: session.baseAccessURL!.appending(path: "templates")) {
if state.hasSession {
Link(destination: state.baseAccessURL!.appending(path: "templates")) {
ButtonRowView {
Text("Create workspace")
}
@@ -62,7 +62,7 @@ struct VPNMenu<VPN: VPNService, S: Session>: View {
ButtonRowView { Text("Approve in System Settings") }
}.buttonStyle(.plain)
} else {
AuthButton<VPN, S>()
AuthButton<VPN>()
}
Button {
openSettings()
@@ -88,13 +88,13 @@ struct VPNMenu<VPN: VPNService, S: Session>: View {
}.padding([.horizontal, .bottom], Theme.Size.trayMargin)
}.padding(.bottom, Theme.Size.trayMargin)
.environmentObject(vpn)
.environmentObject(session)
.environmentObject(state)
.onReceive(inspection.notice) { inspection.visit(self, $0) } // ViewInspector
}

private var vpnDisabled: Bool {
waitCleanup ||
!session.hasSession ||
!state.hasSession ||
vpn.state == .connecting ||
vpn.state == .disconnecting ||
vpn.state == .failed(.systemExtensionError(.needsUserApproval))
@@ -120,8 +120,8 @@ func openSystemExtensionSettings() {

#if DEBUG
#Preview {
VPNMenu<PreviewVPN, PreviewSession>().frame(width: 256)
VPNMenu<PreviewVPN>().frame(width: 256)
.environmentObject(PreviewVPN())
.environmentObject(PreviewSession())
.environmentObject(AppState(persistent: false))
}
#endif
6 changes: 3 additions & 3 deletions Coder Desktop/Coder Desktop/Views/VPNState.swift
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import SwiftUI

struct VPNState<VPN: VPNService, S: Session>: View {
struct VPNState<VPN: VPNService>: View {
@EnvironmentObject var vpn: VPN
@EnvironmentObject var session: S
@EnvironmentObject var state: AppState

let inspection = Inspection<Self>()

var body: some View {
Group {
switch (vpn.state, session.hasSession) {
switch (vpn.state, state.hasSession) {
case (.failed(.systemExtensionError(.needsUserApproval)), _):
Text("Awaiting System Extension approval")
.font(.body)
11 changes: 6 additions & 5 deletions Coder Desktop/Coder DesktopTests/AgentsTests.swift
Original file line number Diff line number Diff line change
@@ -7,15 +7,16 @@ import ViewInspector
@Suite(.timeLimit(.minutes(1)))
struct AgentsTests {
let vpn: MockVPNService
let session: MockSession
let sut: Agents<MockVPNService, MockSession>
let state: AppState
let sut: Agents<MockVPNService>
let view: any View

init() {
vpn = MockVPNService()
session = MockSession()
sut = Agents<MockVPNService, MockSession>()
view = sut.environmentObject(vpn).environmentObject(session)
state = AppState(persistent: false)
state.login(baseAccessURL: URL(string: "https://coder.example.com")!, sessionToken: "fake-token")
sut = Agents<MockVPNService>()
view = sut.environmentObject(vpn).environmentObject(state)
}

private func createMockAgents(count: Int, status: AgentStatus = .okay) -> [UUID: Agent] {
Original file line number Diff line number Diff line change
@@ -15,7 +15,7 @@ struct LiteralHeadersSettingTests {
sut = LiteralHeadersSection<MockVPNService>()
let store = UserDefaults(suiteName: #file)!
store.removePersistentDomain(forName: #file)
view = sut.environmentObject(vpn).environmentObject(Settings(store: store))
view = sut.environmentObject(vpn).environmentObject(AppState(persistent: false))
}

@Test
12 changes: 6 additions & 6 deletions Coder Desktop/Coder DesktopTests/LoginFormTests.swift
Original file line number Diff line number Diff line change
@@ -8,16 +8,16 @@ import ViewInspector
@MainActor
@Suite(.timeLimit(.minutes(1)))
struct LoginTests {
let session: MockSession
let sut: LoginForm<MockSession>
let state: AppState
let sut: LoginForm
let view: any View

init() {
session = MockSession()
sut = LoginForm<MockSession>()
state = AppState(persistent: false)
sut = LoginForm()
let store = UserDefaults(suiteName: #file)!
store.removePersistentDomain(forName: #file)
view = sut.environmentObject(session).environmentObject(Settings(store: store))
view = sut.environmentObject(state)
}

@Test
@@ -120,7 +120,7 @@ struct LoginTests {
try view.find(ViewType.SecureField.self).setInput("valid-token")
try await view.actualView().submit()

#expect(session.hasSession)
#expect(state.hasSession)
}
}
}
25 changes: 0 additions & 25 deletions Coder Desktop/Coder DesktopTests/Util.swift
Original file line number Diff line number Diff line change
@@ -25,29 +25,4 @@ class MockVPNService: VPNService, ObservableObject {
func configureTunnelProviderProtocol(proto _: NETunnelProviderProtocol?) {}
}

class MockSession: Session {
@Published
var hasSession: Bool = false
@Published
var sessionToken: String? = "fake-token"
@Published
var baseAccessURL: URL? = URL(string: "https://dev.coder.com")!

func store(baseAccessURL _: URL, sessionToken _: String) {
hasSession = true
baseAccessURL = URL(string: "https://dev.coder.com")!
sessionToken = "fake-token"
}

func clear() {
hasSession = false
sessionToken = nil
baseAccessURL = nil
}

func tunnelProviderProtocol() -> NETunnelProviderProtocol? {
nil
}
}

extension Inspection: @unchecked Sendable, @retroactive InspectionEmissary {}
15 changes: 7 additions & 8 deletions Coder Desktop/Coder DesktopTests/VPNMenuTests.swift
Original file line number Diff line number Diff line change
@@ -7,21 +7,19 @@ import ViewInspector
@Suite(.timeLimit(.minutes(1)))
struct VPNMenuTests {
let vpn: MockVPNService
let session: MockSession
let sut: VPNMenu<MockVPNService, MockSession>
let state: AppState
let sut: VPNMenu<MockVPNService>
let view: any View

init() {
vpn = MockVPNService()
session = MockSession()
sut = VPNMenu<MockVPNService, MockSession>()
view = sut.environmentObject(vpn).environmentObject(session)
state = AppState(persistent: false)
sut = VPNMenu<MockVPNService>()
view = sut.environmentObject(vpn).environmentObject(state)
}

@Test
func testVPNLoggedOut() async throws {
session.hasSession = false

try await ViewHosting.host(view) {
try await sut.inspection.inspect { view in
let toggle = try view.find(ViewType.Toggle.self)
@@ -104,7 +102,8 @@ struct VPNMenuTests {

@Test
func testOffWhenFailed() async throws {
session.hasSession = true
state.login(baseAccessURL: URL(string: "https://coder.example.com")!, sessionToken: "fake-token")

try await ViewHosting.host(view) {
try await sut.inspection.inspect { view in
let toggle = try view.find(ViewType.Toggle.self)
12 changes: 6 additions & 6 deletions Coder Desktop/Coder DesktopTests/VPNStateTests.swift
Original file line number Diff line number Diff line change
@@ -7,16 +7,16 @@ import ViewInspector
@Suite(.timeLimit(.minutes(1)))
struct VPNStateTests {
let vpn: MockVPNService
let session: MockSession
let sut: VPNState<MockVPNService, MockSession>
let state: AppState
let sut: VPNState<MockVPNService>
let view: any View

init() {
vpn = MockVPNService()
sut = VPNState<MockVPNService, MockSession>()
session = MockSession()
session.hasSession = true
view = sut.environmentObject(vpn).environmentObject(session)
sut = VPNState<MockVPNService>()
state = AppState(persistent: false)
state.login(baseAccessURL: URL(string: "https://coder.example.com")!, sessionToken: "fake-token")
view = sut.environmentObject(vpn).environmentObject(state)
}

@Test