feat: same-origin cookie/SSO authentication mode (additive, dual-mode) - #1091
Open
mairas wants to merge 17 commits into
Open
feat: same-origin cookie/SSO authentication mode (additive, dual-mode)#1091mairas wants to merge 17 commits into
mairas wants to merge 17 commits into
Conversation
The login password was stored in plaintext in connectionConfig and re-POSTed on token renewal. Remove that persistence: - loginPassword is now transient (in-memory only); buildConnectionStorageObject omits it and loadConnectionConfig idempotently strips any legacy persisted value (no connectionConfig version bump). - Option A: the constructor keeps an unexpired user-session JWT on startup (mirroring device tokens) so the expiring JWT is the cross-reload credential. - Token renewal no longer re-POSTs stored credentials (auth/validate is unimplemented on the server); on expiry it deletes the token to surface re-login. - connectToServer establishes the session in-memory before reload. Adds a localStorage test helper for the unit-test runner (jsdom opaque origin has no Web Storage). Service-level specs cover the persistence and renewal behavior; the connectToServer flow is covered by the deploy-time acceptance test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ession (Unit 2)
Add a synchronous `authMode` ('cookie' | 'token') derived from the connection
config (proxy-aware, origin-compared), available before connection discovery so
the interceptor/bootstrap can branch from the first request. Cookie mode = KIP
served same-origin as the SK server.
In cookie mode the constructor ignores a stored user token (the SSO/session
cookie is authoritative) but keeps a stored device token as the unattended
same-origin fallback (not stranded). Token mode is unchanged (Option A: keep the
unexpired user JWT). Carriage changes land in Unit 4.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add refreshLoginStatus() — a cookie-mode-only credentialed GET to /skServer/loginStatus that derives session state fail-closed (logged in only when status === 'loggedIn'); token mode does not consult it. Expose loginStatus$ (OIDC/auth descriptors the bootstrap redirect needs) plus two derived signals: isUserSession$ (a real per-user identity) and canWriteUserData$ (a user session that is not server-side read-only), both combineLatest over authToken$ + loginStatus$ branching on authMode. Unit 3 of the Signal K standard-auth plan (R2, R3, R13). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rame Branch all three credential carriers on auth mode first: - HTTP interceptor: cookie mode sends withCredentials and no JWT header (even if a stale token is stored); token mode keeps the header. - WebSocket: extract buildWebSocketUrl and omit &token= in cookie mode; drive the (re)connect off the isLoggedIn$ transition (the authToken$ path is dead in cookie mode), reusing the isFullyConnected guard to avoid a double-connect with the bootstrap; re-check loginStatus on a non-clean drop so an expired cookie surfaces as a logout. - Freeboard iframe: build the src from window.location.origin with no ?token= in cookie mode (proxy leaves signalKUrl cross-origin), via a pure buildFreeboardSkUrl helper. Extend the global AuthenticationService test stub with the Unit 3 session surface (authMode, loginStatus$, isUserSession$, canWriteUserData$, refreshLoginStatus). Unit 4 of the Signal K standard-auth plan (R4). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…onfig Add a useServerStorage decision (cookie mode OR useSharedConfig) and re-key every SettingsService storage-routing branch and the startup() load to it, plus the app-init remote-bootstrap gate. In cookie mode config now persists to the server applicationData slot regardless of the stored useSharedConfig flag — the same decoupling applied to auth carriage — closing the cookie-mode localStorage split-brain. Unit 5 of the Signal K standard-auth plan (R12). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drive the cookie-mode bootstrap from loginStatus: logged in proceeds to the storage bootstrap; not-logged-in + authRequired auto-redirects to the SK/SSO login (oidcLoginUrl, else the admin login) honoring oidcAutoLogin; auth-not-required is anonymous read with no redirect. A reload-surviving sessionStorage budget (SsoRedirectService) caps auto-redirect attempts so a kiosk with oidcAutoLogin cannot loop: - the budget resets only on a genuinely completed, authorized bootstrap, so a loggedIn -> applicationData-401 -> reauth path stays bounded; - an auth-blocked outcome (budget exhausted, oidcAutoLogin:false, or a null/unreachable loginStatus) finishes the bootstrap degraded, preserving the auth-blocked recovery state (and its Sign-in toast) without resetting the budget; - it fails closed (no auto-redirect) when sessionStorage is unavailable or silently discards writes, rather than fail open into a loop; - the mid-bootstrap 401 path reuses the same oidcAutoLogin/budget-guarded decision. An explicit Sign in bypasses the budget and sets noAutoLogin. returnTo is validated relative-only (reject //, backslash, control chars, cross-origin, self-route) to prevent an open redirect. The bootstrap starts the WebSocket only from a fresh HTTPConnected state, so it does not double-connect with the cookie isLoggedIn$ reconnect. Convert the bootstrap /login navigations to the mode-aware path and surface an auth-blocked recovery toast (Sign in) in cookie mode. Unit 6 of the Signal K standard-auth plan (R1, R3, R9, R11). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In cookie mode the /login route no longer opens KIP's credential dialog — it redirects to the SK/SSO login and shows a transitional state. The Connectivity tab hides the credential controls and shows a session identity block (signed-in/provider, read-only note, anonymous, sign-in, loading); connectToServer skips the credential dialog/login and clears a stale user token when the new config resolves to cookie mode. Refactor effectiveOriginIsSameAsApp into a public authModeForConfig so the Connectivity tab can resolve the mode of a config being edited. Unit 7 of the Signal K standard-auth plan (R1, R5, R11, R13). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Document the dual-mode auth feature (same-origin SSO, password-storage removal, cross-origin re-login on expiry). Correct the plan's stale "re-login on each reload" note — Option A persists the session JWT across reloads, so token-mode re-login is on expiry only. Unit 9 of the Signal K standard-auth plan. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…yAccess flag canWriteUserData was derived from loginStatus.readOnlyAccess, but that field is the server's allow_readonly (anonymous read) config — true on a server that permits anonymous read — not the signed-in user's permission. A signed-in admin was therefore shown "Read-only access". Derive write capability from userLevel instead (admin/readwrite can write; readonly cannot), matching Signal K's own permission check. Resolves the userLevel/readOnlyAccess write-gating question the plan deferred to deploy-time. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lyAccess Review #1: refreshLoginStatus() now pipes timeout(5000) so a hung /skServer/loginStatus cannot block the APP_INITIALIZER from rendering the app (every sibling bootstrap GET already does this); a TimeoutError lands in the existing fail-closed catch. Review mxtommy#8: add canWriteUserData tests for userLevel 'readwrite' (write), missing userLevel (fail closed), plus the timeout fail-closed case. Review mxtommy#11/mxtommy#16: remove the dead readOnlyAccess field, renewToken() and the auth/validate URL (404, no caller); record in the plan that the write gate keys off userLevel (admin/readwrite), not the server allow_readonly flag, and add the deferred review follow-ups. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review #1: getConfig() pipes timeout(5000) so a hung applicationData fetch cannot stall the bootstrap after waitUntilReady() passes. Review #2b: wrap the patchQueue concatMap in catchError -> EMPTY so one failed patch (e.g. a read-only session's 401) no longer terminates the queue and silently drops every later config save. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review mxtommy#6: in cookie mode the interceptor attached withCredentials to every request, including the cross-origin discovery GET under proxyEnabled + cross-origin signalKUrl (which depends on the foreign server's CORS allow-credentials and can fail the connection). Send credentials only on same-origin requests; a cross-origin request gets neither credentials nor a token header. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d path Review mxtommy#7: the bootstrap finally started the WebSocket whenever HTTP was connected, even on the cookie auth-blocked path (HTTP connected but no session) -> an anonymous WS that churns behind the recovery toast. Start the WS only on a clean (non-degraded, non-redirecting) bootstrap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review #4b: a missing server config returns {} (not 404), which passed
the === null guard and made pushSettings() dereference activeConfig.app
-> TypeError. Guard on the presence of app config instead.
Review mxtommy#17: collapse the asymmetric useDeviceToken if/else to
useDeviceToken = !useSharedConfig.
Review mxtommy#15: seed the local config keys in the storage-routing tests so the
localStorage startup() branch loads cleanly instead of throwing an
unhandled, suite-masking JSON parse error.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ceeds Review mxtommy#12: connectToServer persisted the new config before the in-memory login could fail, leaving a rejected config persisted (applied on the next reload) with no rollback. Run the login first, then persist. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review mxtommy#13: isSafeReturnTo validated the raw string, so '/a/..//evil' (which resolves to '//evil') was accepted. Reject when the resolved pathname is protocol-relative, holding the open-redirect invariant on the normalized path. Not reachable today (the only caller passes the browser-normalized location.pathname); hardens against future callers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review mxtommy#14: the global AuthenticationServiceStub lacked authModeForConfig, loginStatusValue and deleteToken, and AppNetworkInitServiceStub's bootstrapIssue$ type omitted the new 'auth-blocked' reason and cause, so a future component spec exercising those paths would drift undetected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
KIP manages its own username/password login and JWT. When KIP is served by a Signal K server that authenticates via OIDC/SSO, users logged in through SSO have no SK-local password, so KIP's login form can't authenticate them — they hit a second login they can't satisfy.
This adds the Signal K-documented embedded-webapp auth pattern as an opt-in, auto-detected mode, without removing the existing token model.
Approach (dual-mode, detected by origin)
GET /skServer/loginStatus; when not logged in, KIP redirects to the login URL loginStatus advertises (OIDC when enabled, else admin login). No JWT in localStorage and no token in the WS query string — the cookie authorizes REST, the WS upgrade, and the embedded Freeboard iframe.Backward compatibility
Additive: cross-origin/standalone behavior is unchanged, and same-origin-without-SSO still works via the cookie session. The "logged-in"/storage-routing gating moves from token-presence to a loginStatus/session notion so cookie sessions are first-class.
Security hardening (independent of mode)
withCredentialsscoped to same-origin requests; bootstrap network calls are timeout-bounded; the config patch queue survives a failed write; open-redirect guard on the SSO return URL.Testing
Unit tests (Vitest) for mode detection, interceptor behavior in both modes, loginStatus-driven state, the SSO redirect budget/recovery, and the return-URL guard. Verified live against signalk-server v2.27.0 behind OIDC SSO: cookie authorizes REST + WS with no stored JWT, and not-logged-in triggers the SSO redirect.
Note: ~50 component specs fail pre-existing under the unit runner due to a jsdom localStorage limitation unrelated to this change.
🤖 Generated with Claude Code