From 7868cf835116125e3a140eddd7e47e1f3af3368c Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Thu, 16 Jul 2026 18:33:25 +0200 Subject: [PATCH 1/3] Surface OAuth failures on server selection + preserve HA base path Two onboarding fixes from the code audit: - ServerSelectionView now renders viewModel.errorMessage via an alert, so a failed OAuth prepare (server card) or a failed token exchange in handleOAuthCallback is visible and dismissible instead of silently returning the user to the server list. The alert is gated to only present when no sheet is up; the manual-entry sheet keeps its inline validation text and clears the error on disappear so a manual-validation message never leaks into the alert. errorMessage is cleared when opening manual entry (prepareManualEntry). - OAuthFlowManager.prepare appends the callback/authorize paths onto the server URL's existing base path instead of overwriting it, so subpath-hosted HA (e.g. https://example.com/ha) builds correct same-origin URLs. Force-unwraps on the runtime URLComponents/URL are replaced with guard-let throwing AuthError.invalidServerURL. Co-Authored-By: Claude Opus 4.8 --- Hemera/Auth/OAuthFlowManager.swift | 23 +++++++-- Hemera/UI/Onboarding/ManualEntrySheet.swift | 1 + .../UI/Onboarding/ServerSelectionView.swift | 18 ++++++- .../Onboarding/ServerSelectionViewModel.swift | 4 ++ HemeraTests/Auth/OAuthFlowManagerTests.swift | 50 +++++++++++++++++++ .../ServerSelectionViewModelTests.swift | 40 +++++++++++++++ 6 files changed, 130 insertions(+), 6 deletions(-) create mode 100644 HemeraTests/Auth/OAuthFlowManagerTests.swift diff --git a/Hemera/Auth/OAuthFlowManager.swift b/Hemera/Auth/OAuthFlowManager.swift index 402c9b6..ac46790 100644 --- a/Hemera/Auth/OAuthFlowManager.swift +++ b/Hemera/Auth/OAuthFlowManager.swift @@ -24,14 +24,27 @@ final class OAuthFlowManager { let state = UUID().uuidString // Same-origin redirect URI — HA accepts without fetching client_id - var redirectComponents = URLComponents(url: serverURL, resolvingAgainstBaseURL: false)! - redirectComponents.path = "/hemera_callback" + guard var redirectComponents = URLComponents(url: serverURL, resolvingAgainstBaseURL: false) else { + throw AuthError.invalidServerURL + } + + // Preserve any base path (e.g. subpath-hosted HA behind a reverse proxy) + // by appending endpoint paths instead of replacing the server URL's path. + let rawBasePath = redirectComponents.path + let basePath = rawBasePath.hasSuffix("/") ? String(rawBasePath.dropLast()) : rawBasePath + + redirectComponents.path = basePath + "/hemera_callback" redirectComponents.queryItems = nil redirectComponents.fragment = nil - let redirectURI = redirectComponents.url!.absoluteString + guard let redirectURL = redirectComponents.url else { + throw AuthError.invalidServerURL + } + let redirectURI = redirectURL.absoluteString - var authComponents = URLComponents(url: serverURL, resolvingAgainstBaseURL: false)! - authComponents.path = "/auth/authorize" + guard var authComponents = URLComponents(url: serverURL, resolvingAgainstBaseURL: false) else { + throw AuthError.invalidServerURL + } + authComponents.path = basePath + "/auth/authorize" authComponents.queryItems = [ URLQueryItem(name: "client_id", value: clientId), URLQueryItem(name: "redirect_uri", value: redirectURI), diff --git a/Hemera/UI/Onboarding/ManualEntrySheet.swift b/Hemera/UI/Onboarding/ManualEntrySheet.swift index 0b0f783..103cf65 100644 --- a/Hemera/UI/Onboarding/ManualEntrySheet.swift +++ b/Hemera/UI/Onboarding/ManualEntrySheet.swift @@ -71,6 +71,7 @@ struct ManualEntrySheet: View { } } .presentationDetents([.medium]) + .onDisappear { viewModel.errorMessage = nil } } private func connectAndDismiss() { diff --git a/Hemera/UI/Onboarding/ServerSelectionView.swift b/Hemera/UI/Onboarding/ServerSelectionView.swift index 357fd88..9c98ca0 100644 --- a/Hemera/UI/Onboarding/ServerSelectionView.swift +++ b/Hemera/UI/Onboarding/ServerSelectionView.swift @@ -33,6 +33,17 @@ struct ServerSelectionView: View { } message: { Text(Localization.unencryptedMessage) } + .alert( + Localization.connectionFailedTitle, + isPresented: Binding( + get: { viewModel.errorMessage != nil && activeSheet == nil }, + set: { if !$0 { viewModel.errorMessage = nil } } + ) + ) { + Button(Localization.ok, role: .cancel) { viewModel.errorMessage = nil } + } message: { + Text(viewModel.errorMessage ?? "") + } .sheet(item: $activeSheet, onDismiss: { if viewModel.authSession != nil { viewModel.cancelAuth() @@ -151,7 +162,10 @@ struct ServerSelectionView: View { iconColor: .blue, title: Localization.enterManually, subtitle: Localization.enterManuallySubtitle, - action: { activeSheet = .manualEntry } + action: { + viewModel.prepareManualEntry() + activeSheet = .manualEntry + } ) { DisclosureIndicator() } @@ -179,6 +193,8 @@ private extension ServerSelectionView { private extension ServerSelectionView { enum Localization { static let title = String(localized: "Connect to Server", comment: "Navigation title for the server connection screen") + static let connectionFailedTitle = String(localized: "Couldn’t Connect", comment: "Alert title shown when connecting to a Home Assistant server fails during onboarding") + static let ok = String(localized: "OK", comment: "Button dismissing the connection-failure alert") static let signIn = String(localized: "Sign In", comment: "Navigation title for the OAuth sign-in screen") static let cancel = String(localized: "Cancel", comment: "Button to cancel the current action") diff --git a/Hemera/UI/Onboarding/ServerSelectionViewModel.swift b/Hemera/UI/Onboarding/ServerSelectionViewModel.swift index fb32bfe..106bef7 100644 --- a/Hemera/UI/Onboarding/ServerSelectionViewModel.swift +++ b/Hemera/UI/Onboarding/ServerSelectionViewModel.swift @@ -39,6 +39,10 @@ final class ServerSelectionViewModel { self.init(authManager: ServiceLocator.shared.authManager) } + func prepareManualEntry() { + errorMessage = nil + } + func startDiscovery() { discovery.startDiscovery() } diff --git a/HemeraTests/Auth/OAuthFlowManagerTests.swift b/HemeraTests/Auth/OAuthFlowManagerTests.swift new file mode 100644 index 0000000..31d1640 --- /dev/null +++ b/HemeraTests/Auth/OAuthFlowManagerTests.swift @@ -0,0 +1,50 @@ +import Foundation +import Testing +@testable import Hemera + +struct OAuthFlowManagerTests { + + // MARK: - No Base Path + + @Test + func prepare_withNoBasePath_buildsRootLevelURLs() throws { + let session = try OAuthFlowManager().prepare(serverURL: URL(string: "https://ha.example.com:8123")!) + + #expect(session.redirectURI == "https://ha.example.com:8123/hemera_callback") + + let authComponents = URLComponents(url: session.authorizeURL, resolvingAgainstBaseURL: false)! + #expect(authComponents.path == "/auth/authorize") + #expect(redirectURI(in: authComponents) == "https://ha.example.com:8123/hemera_callback") + } + + // MARK: - Base Path + + @Test + func prepare_withBasePath_preservesBasePath() throws { + let session = try OAuthFlowManager().prepare(serverURL: URL(string: "https://example.com/ha")!) + + #expect(session.redirectURI == "https://example.com/ha/hemera_callback") + + let authComponents = URLComponents(url: session.authorizeURL, resolvingAgainstBaseURL: false)! + #expect(authComponents.path == "/ha/auth/authorize") + #expect(redirectURI(in: authComponents) == "https://example.com/ha/hemera_callback") + } + + @Test + func prepare_withTrailingSlash_doesNotDoubleSlash() throws { + let session = try OAuthFlowManager().prepare(serverURL: URL(string: "https://example.com/ha/")!) + + #expect(session.redirectURI == "https://example.com/ha/hemera_callback") + + let authComponents = URLComponents(url: session.authorizeURL, resolvingAgainstBaseURL: false)! + #expect(authComponents.path == "/ha/auth/authorize") + } +} + +// MARK: - Helpers + +private extension OAuthFlowManagerTests { + func redirectURI(in components: URLComponents) -> String? { + components.queryItems?.first { $0.name == "redirect_uri" }?.value + } +} diff --git a/HemeraTests/UI/Onboarding/ServerSelectionViewModelTests.swift b/HemeraTests/UI/Onboarding/ServerSelectionViewModelTests.swift index 108d6d2..afd847c 100644 --- a/HemeraTests/UI/Onboarding/ServerSelectionViewModelTests.swift +++ b/HemeraTests/UI/Onboarding/ServerSelectionViewModelTests.swift @@ -114,6 +114,46 @@ struct ServerSelectionViewModelTests { #expect(viewModel.pendingHTTPURL == nil) } + // MARK: - OAuth Callback Error Surfacing + + @Test + func handleOAuthCallback_whenCallbackFails_setsErrorMessageAndDoesNotAuthenticate() async { + let authManager = MockAuthManager() + let viewModel = ServerSelectionViewModel(authManager: authManager) + let serverURL = URL(string: "https://ha.example.com:8123")! + let session = OAuthFlowManager.AuthSession( + authorizeURL: serverURL, + redirectURI: "https://ha.example.com:8123/hemera_callback", + state: "expected-state", + clientId: serverURL.absoluteString, + serverURL: serverURL + ) + // Callback carries a mismatched state, so handleCallback throws before any token exchange. + let callbackURL = URL(string: "https://ha.example.com:8123/hemera_callback?state=wrong&code=abc")! + + await viewModel.handleOAuthCallback(url: callbackURL, session: session) + + #expect(viewModel.errorMessage != nil) + #expect(viewModel.isConnecting == false) + #expect(authManager.didAuthenticateCallCount == 0) + } + + @Test + func prepareManualEntry_clearsStaleErrorMessage() { + let viewModel = makeViewModel() + viewModel.errorMessage = "stale error" + viewModel.prepareManualEntry() + #expect(viewModel.errorMessage == nil) + } + + @Test + func startOAuth_clearsPreviousErrorMessage() { + let viewModel = makeViewModel() + viewModel.errorMessage = "stale error" + viewModel.startOAuth(url: URL(string: "https://ha.example.com:8123")!) + #expect(viewModel.errorMessage == nil) + } + // MARK: - Auth Cancellation @Test From 8803ff74161e0dfc65f596f5a061767436c281ca Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Thu, 16 Jul 2026 21:41:18 +0200 Subject: [PATCH 2/3] Use /** */ block form for the base-path comment Per project comment-style convention: multi-line comments use the /** ... */ block form. Converts the two-line base-path comment introduced in the previous commit; single-line // comments left as-is. Co-Authored-By: Claude Opus 4.8 --- Hemera/Auth/OAuthFlowManager.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Hemera/Auth/OAuthFlowManager.swift b/Hemera/Auth/OAuthFlowManager.swift index ac46790..8587e2b 100644 --- a/Hemera/Auth/OAuthFlowManager.swift +++ b/Hemera/Auth/OAuthFlowManager.swift @@ -28,8 +28,10 @@ final class OAuthFlowManager { throw AuthError.invalidServerURL } - // Preserve any base path (e.g. subpath-hosted HA behind a reverse proxy) - // by appending endpoint paths instead of replacing the server URL's path. + /** + Preserve any base path (e.g. subpath-hosted HA behind a reverse proxy) + by appending endpoint paths instead of replacing the server URL's path. + */ let rawBasePath = redirectComponents.path let basePath = rawBasePath.hasSuffix("/") ? String(rawBasePath.dropLast()) : rawBasePath From 115e4489ff514c80756fb5ddcbb84790b23b2b3a Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Thu, 16 Jul 2026 22:15:49 +0200 Subject: [PATCH 3/3] Keep manual-entry sheet open when a connect attempt fails synchronously MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit connectAndDismiss dismissed the sheet whenever connectManual returned true, even when the subsequent connect()/startOAuth set an error synchronously (e.g. a URL with embedded credentials passes field validation but is rejected by OAuth preparation). The sheet's onDisappear then cleared errorMessage before the parent alert could present, so the attempt failed silently with no error shown anywhere. Now connectAndDismiss only dismisses when the attempt produced no error, keeping the inline error visible for all synchronous manual-entry failures — consistent with existing field-validation errors. Co-Authored-By: Claude Opus 4.8 --- Hemera/UI/Onboarding/ManualEntrySheet.swift | 11 +++++++++-- .../Onboarding/ServerSelectionViewModelTests.swift | 13 +++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/Hemera/UI/Onboarding/ManualEntrySheet.swift b/Hemera/UI/Onboarding/ManualEntrySheet.swift index 103cf65..d272d39 100644 --- a/Hemera/UI/Onboarding/ManualEntrySheet.swift +++ b/Hemera/UI/Onboarding/ManualEntrySheet.swift @@ -75,9 +75,16 @@ struct ManualEntrySheet: View { } private func connectAndDismiss() { - if viewModel.connectManual() { - onDismiss() + /** + Keep the sheet open if the attempt failed synchronously (e.g. an + invalid URL that passes field validation but is rejected by OAuth + preparation) so the inline error stays visible instead of the sheet + dismissing over an unshown error. + */ + guard viewModel.connectManual(), viewModel.errorMessage == nil else { + return } + onDismiss() } } diff --git a/HemeraTests/UI/Onboarding/ServerSelectionViewModelTests.swift b/HemeraTests/UI/Onboarding/ServerSelectionViewModelTests.swift index afd847c..0473fc4 100644 --- a/HemeraTests/UI/Onboarding/ServerSelectionViewModelTests.swift +++ b/HemeraTests/UI/Onboarding/ServerSelectionViewModelTests.swift @@ -138,6 +138,19 @@ struct ServerSelectionViewModelTests { #expect(authManager.didAuthenticateCallCount == 0) } + @Test + func connectManual_withCredentialsInURL_returnsTrueButSetsError() { + // The URL passes field validation but OAuth preparation rejects the + // embedded credentials, so connectManual returns true yet leaves an + // error and no session — the manual sheet relies on this to stay open. + let viewModel = makeViewModel() + viewModel.manualURL = "https://user:pass@ha.example.com" + let result = viewModel.connectManual() + #expect(result == true) + #expect(viewModel.errorMessage != nil) + #expect(viewModel.authSession == nil) + } + @Test func prepareManualEntry_clearsStaleErrorMessage() { let viewModel = makeViewModel()