Skip to content

Surface OAuth failures on server selection & support subpath-hosted HA - #24

Merged
adborbas merged 3 commits into
mainfrom
fix/oauth-onboarding
Jul 16, 2026
Merged

Surface OAuth failures on server selection & support subpath-hosted HA#24
adborbas merged 3 commits into
mainfrom
fix/oauth-onboarding

Conversation

@adborbas

Copy link
Copy Markdown
Owner

Changes

  • Render viewModel.errorMessage on ServerSelectionView via an .alert, so a failed OAuth prepare (from a discovered/manually-entered server) or a failed token exchange in handleOAuthCallback is now visible and dismissible instead of silently returning the user to the server list. The alert is gated to present only when no sheet is up.
  • ManualEntrySheet keeps its inline validation text and clears errorMessage on disappear so a manual-validation message never leaks into the parent alert. connectAndDismiss only dismisses when the attempt produced no error, so all synchronous manual-entry failures (including a URL that passes field validation but is rejected by OAuth prep) stay visible inline rather than dismissing over an unshown error.
  • errorMessage is cleared when opening manual entry (prepareManualEntry).
  • OAuthFlowManager.prepare now appends the callback/authorize paths onto the server URL's existing base path instead of overwriting it, so subpath-hosted HA behind a reverse proxy (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.

Why

Two code-audit findings:

  • OAuth/connection failures were surfaced nowhere on the server-selection screen — errorMessage was set on two failure paths but only ever rendered inside the manual-entry sheet, so a failed connect looked like it silently did nothing.
  • OAuth authorize/redirect URLs overwrote the server's base path, breaking login for HA instances served under a URL subpath (the app requested https://example.com/auth/authorize instead of https://example.com/ha/auth/authorize).

Notes

  • No-base-path URLs remain byte-identical to before; base-path and trailing-slash cases are covered by new OAuthFlowManagerTests.
  • New view-model tests cover the OAuth-callback failure surfacing, stale-error clearing, and the "returns true but sets error" contract that connectAndDismiss relies on.
  • Demo mode is unaffected (the OAuth flow is bypassed via "Try Demo").
  • Not reproducible in demo/simulator without a live HA server; verified via clean build + HemeraTests (all green).

adborbas and others added 3 commits July 16, 2026 18:33
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
@adborbas
adborbas merged commit 67df327 into main Jul 16, 2026
1 check passed
@adborbas
adborbas deleted the fix/oauth-onboarding branch July 16, 2026 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant