diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c02fbc92..90deb8d3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ Versions follow the `version+build` scheme from `pubspec.yaml`, bumped via - **Library consumers:** `ThreadStateWarning.sourcesSkipped`, `RagSnapshot.carriesUnreadableCitations` and `RagSnapshot.withUnreadableCitationsDropped`. +- **Library consumers:** `CallbackParams`, the type of `standardFlavor`'s + `callbackParams`, is exported. ### Changed @@ -30,6 +32,13 @@ Versions follow the `version+build` scheme from `pubspec.yaml`, bumped via - The account name and email come from the sign-in tokens instead of a `/api/user_info` request, so they show as soon as the app opens. The lobby and the room rail show the email once when it is also the name or username. +- After a web sign-in, the tokens leave the address bar before the app starts + loading rather than once it has booted. Forks with their own + `web/index.html` need the script in `docs/authoring-a-flavor.md`; without it + sign-in still works and the app logs an error. +- An auto-connect address (`?url=`, from the app's own sign-in prompts or an + iOS deep link) connects only to a server already in the app's list, at the + address it was added with; any other address is ignored. - **Library consumers:** `AuthProviderConfig.scope` is `String?`. ### Removed @@ -39,6 +48,9 @@ Versions follow the `version+build` scheme from `pubspec.yaml`, bumped via they now pass through unprocessed. Reasoning an older backend stored as `THINKING_*` events no longer shows when such a thread is reopened, and a `MESSAGES_SNAPSHOT` no longer logs a warning. +- Web sign-in against backends that return the tokens in the query string + (before `v0.82.2` / `v0.83.2`); those put the tokens in the logs of the + server hosting the app. - **Library consumers (breaking):** `StepProgress` removed from `ExecutionEvent`; `bridgeBaseEvent` maps `STEP_STARTED` and every `THINKING_*` event to `null`, and `processEvent` leaves the conversation and @@ -89,6 +101,8 @@ Versions follow the `version+build` scheme from `pubspec.yaml`, bumped via closed no longer throws. - A failed history refresh over loaded messages is recorded in the diagnostics log, by thread and type of failure. +- A web sign-in link whose query can't be decoded shows an error instead of + stopping the app from starting. ## [0.107.0+93] - 2026-09-29 diff --git a/docs/authoring-a-flavor.md b/docs/authoring-a-flavor.md index 7f9cf130b..1d454824c 100644 --- a/docs/authoring-a-flavor.md +++ b/docs/authoring-a-flavor.md @@ -37,7 +37,7 @@ extension and runs the contrast check. import 'package:flutter/material.dart'; import 'package:soliplex_frontend/soliplex_frontend.dart'; -Future myFlavor() { +Future myFlavor({required CallbackParams callbackParams}) { final light = buildSoliplexThemeData( colors: lightSoliplexColors.copyWith(primary: const Color(0xFF0A7AFF)), brightness: Brightness.light); @@ -52,6 +52,7 @@ Future myFlavor() { ), defaultBackendUrl: 'https://api.mybrand.com', theme: FlavorTheme.themeData(light: light, dark: dark), + callbackParams: callbackParams, // Custom modules receive the composition kit, so they can share the // standard flavor's session state: // extraModules: (kit) => [MyCustomModule(kit.serverManager)], @@ -61,7 +62,9 @@ Future myFlavor() { Future main() async { WidgetsFlutterBinding.ensureInitialized(); installLogSinks(); - final flavor = await myFlavor(); + final callbackParams = CallbackParamsCapture.captureNow(); + clearCallbackUrl(); + final flavor = await myFlavor(callbackParams: callbackParams); // Pass the builder, not the built config: `Flavor.build()` throws on an // invalid configuration, and a throw out here lands before any view exists — // which on iOS, macOS and Android is not a crash but a launch that never @@ -125,6 +128,43 @@ exported for exactly this, and it is what the shell runs, so your tests cannot drift from production by reproducing the composition slightly differently. Prefer `standardFlavor` unless you genuinely need a different module graph. +## Web entry page + +A fork with its own `web/index.html` must carry the sign-in callback script, +right after ``, with no stylesheet `<link>` or `<script src>` above it, +since either would hold it back until that file downloads: + +```html +<script> + (function () { + var route = '#/auth/callback'; + var hash = window.location.hash; + if (hash.indexOf(route + '?') !== 0) return; + window.soliplexCallbackQuery = hash.substring(route.length + 1); + history.replaceState( + null, '', + window.location.origin + window.location.pathname + + window.location.search + route); + })(); +</script> +``` + +After a web sign-in the backend sends the browser to +`#/auth/callback?token=…`. The script takes the tokens out of the address bar +before the app starts downloading, instead of leaving them there until Flutter +boots. Without it, sign-in still works, but the tokens stay visible for that +time and the app logs an error that names `web/index.html` and this document. + +The browser still records the callback URL, tokens included, in its history. +In Chrome, placing the script after `<title>` makes the history list show the +page title rather than the tokens; only a backend change keeps them out of the +URL. + +`main()` must call `CallbackParamsCapture.captureNow()`, then +`clearCallbackUrl()`, and pass the result to `standardFlavor` as +`callbackParams`, as the example above does. Call them after +`installLogSinks()`: without a sink, the missing-script error is discarded. + ## Rules - Build the theme with `buildSoliplexThemeData` (never a bare `ThemeData`) — the @@ -233,8 +273,11 @@ Prefer `standardFlavor` unless you genuinely need a different module graph. - Declaring a path public leaves your `initialRoute` untouched, so a cold launch lands where it did unless you also set `signedOutLandingPath`. On web a URL to a declared path opens it directly, because go_router prefers a non-`/` - platform route over `initialLocation`; native deep links do not, since no - platform in this repo enables Flutter deep linking. + platform route over `initialLocation`. On iOS, Flutter deep linking is on unless + `Info.plist` sets `FlutterDeepLinkingEnabled` to false, and the app registers + the `ai.soliplex.client` URL scheme, so `ai.soliplex.client:///<path>` reaches + a declared path after the first frame, once the app has built at its initial + route. - A module needs no `go_router` dependency of its own, in `dependencies` or in `dev_dependencies`. The barrel re-exports the routing types module authoring and module *testing* use — `GoRoute`, `GoRouter`, `GoRouterHelper`, diff --git a/docs/developer-setup.md b/docs/developer-setup.md index df228591f..9f704c9ca 100644 --- a/docs/developer-setup.md +++ b/docs/developer-setup.md @@ -170,6 +170,42 @@ Requires Visual Studio with "Desktop development with C++" workload: flutter run -d windows ``` +## Backend sign-in settings + +These live in the backend's OIDC config (`oidc/config.yaml` in the installation, +or each path in its `oidc_paths`) or, where noted, in the identity provider's +client settings. They're not in this repo, but the app depends on them. In +`oidc/config.yaml`, `scope` and `accepted_azp_list` are set per auth system, +under `auth_systems`; `allowed_frontend_origins` and +`unlisted_frontend_origin` may also be set once at the top of the file, for +every auth system in it. + +- `scope` must include `openid`. Without it the identity provider issues no ID + token: sign-out can't name the session to end, and the account name and + identity fall back to the access token. +- Web sign-in from an origin other than the backend's own asks the user to + confirm on a backend page, unless the origin is listed in + `allowed_frontend_origins`. With `unlisted_frontend_origin: deny-all` it + fails with a 400 instead. An unlisted plain `http://` origin that isn't + loopback (such as `localhost` or `127.0.0.1`) fails with a 400 under either + policy. This affects a web build hosted apart from the backend and + `flutter run -d chrome`, whose port makes it a different origin: ask the + backend's operator to list the origin, or confirm the page each time. + Cancelling on that page leaves you on the backend's page, which never sends + you back; return to the app yourself. +- Native apps sign in with the backend's own `client_id` (from `/api/login`), + so their tokens carry it as `azp`. The backend accepts only the + `accepted_azp_list` values (default: that `client_id`); a deployment that + sets the list must keep that `client_id` in it, or native sign-in succeeds + at the identity provider and then every request is refused. +- In the identity provider's client settings, the web app's origin must be + allowed for cross-origin requests (Keycloak: the client's **Web Origins**, + with the app's origin, or `+` when the app is served from the backend's + origin). On web, token refresh is a browser request straight to the + provider's token endpoint. Without that setting, every refresh fails as a + network error, the session is never renewed, and it ends only when the + backend refuses a request with an expired token. + ## Troubleshooting ### Analyzer errors that don't match the code diff --git a/lib/soliplex_frontend.dart b/lib/soliplex_frontend.dart index 0bbe17077..bb8c28b3a 100644 --- a/lib/soliplex_frontend.dart +++ b/lib/soliplex_frontend.dart @@ -100,6 +100,6 @@ export 'src/modules/auth/auth_providers.dart' export 'src/modules/auth/inactivity_logout_storage.dart' show InactivityLogoutFlagStorage, LocalInactivityLogoutFlagStorage; export 'src/modules/auth/platform/callback_service.dart' - show CallbackParamsCapture, clearCallbackUrl; + show CallbackParams, CallbackParamsCapture, clearCallbackUrl; export 'src/modules/auth/consent_notice.dart' show ConsentNotice; export 'src/modules/auth/server_manager.dart' show ServerManager; diff --git a/lib/src/modules/auth/platform/callback_params.dart b/lib/src/modules/auth/platform/callback_params.dart index 41351546e..9b5ac1b15 100644 --- a/lib/src/modules/auth/platform/callback_params.dart +++ b/lib/src/modules/auth/platform/callback_params.dart @@ -17,8 +17,9 @@ class WebCallbackSuccess extends CallbackParams { final int? expiresIn; /// OIDC ID Token. Required as `id_token_hint` for RP-Initiated - /// Logout to deterministically end the IdP SSO session. Null until - /// the BFF includes `id_token` in the callback redirect. + /// Logout to deterministically end the IdP SSO session. Null when the + /// callback carries no ID token: the backend predates v0.82.3 / v0.83.3, its + /// `scope` lacks `openid`, or the provider returned none. final String? idToken; @override @@ -42,6 +43,14 @@ class WebCallbackError extends CallbackParams { String toString() => 'WebCallbackError(error: $error)'; } +/// A web sign-in callback whose query could not be decoded. +class WebCallbackMalformed extends CallbackParams { + const WebCallbackMalformed(); + + @override + String toString() => 'WebCallbackMalformed()'; +} + /// No callback parameters detected. class NoCallbackParams extends CallbackParams { const NoCallbackParams(); diff --git a/lib/src/modules/auth/platform/callback_params_parser.dart b/lib/src/modules/auth/platform/callback_params_parser.dart index 0b41cdeb9..133ed80cc 100644 --- a/lib/src/modules/auth/platform/callback_params_parser.dart +++ b/lib/src/modules/auth/platform/callback_params_parser.dart @@ -1,10 +1,14 @@ +import 'package:soliplex_logging/soliplex_logging.dart'; + import 'callback_params.dart'; +final Logger _logger = LogManager.instance.getLogger('soliplex.auth_callback'); + /// Parses OAuth callback parameters from URL query params. /// /// Returns [WebCallbackError] if an `error` key is present, -/// [WebCallbackSuccess] if a token is found (checks both `token` -/// and `access_token` keys), or [NoCallbackParams] otherwise. +/// [WebCallbackSuccess] if a token is found (checks the `token` key), or +/// [NoCallbackParams] otherwise. CallbackParams parseCallbackParams(Map<String, String> params) { if (params.isEmpty) return const NoCallbackParams(); @@ -16,7 +20,7 @@ CallbackParams parseCallbackParams(Map<String, String> params) { ); } - final accessToken = params['token'] ?? params['access_token']; + final accessToken = params['token']; if (accessToken != null) { return WebCallbackSuccess( accessToken: accessToken, @@ -29,26 +33,78 @@ CallbackParams parseCallbackParams(Map<String, String> params) { return const NoCallbackParams(); } -/// Extracts query parameters from URL search string and hash fragment. -/// -/// Checks [search] first (standard `?key=val`), then falls back to -/// hash-based query params (`#/path?key=val`). -Map<String, String> extractQueryParams({ +/// The sign-in callback route, where the backend puts the tokens after a `?`. +const authCallbackHash = '#/auth/callback'; + +/// The query of a sign-in callback [hash], or `null` when [hash] is not +/// `#/auth/callback?…`. +String? callbackQueryFromHash(String hash) { + const prefix = '$authCallbackHash?'; + return hash.startsWith(prefix) ? hash.substring(prefix.length) : null; +} + +/// The URL to replace the page's with on a page load, so it starts with no +/// query, not in the address bar ([search]) and not in the hash route, or +/// `null` when it carries none. The callback query holds tokens; any other +/// query is set by in-app navigation, so a page load starts with no query: an +/// outside link can't supply one that way. It starts with [origin], so a +/// [pathname] that reads as another host (`//evil.example/`) cannot make it +/// cross-origin. +String? urlWithoutQueries({ + required String origin, + required String pathname, required String search, required String hash, }) { - if (search.isNotEmpty) { - return Uri.splitQueryString(search.substring(1)); - } + final queryStart = hash.indexOf('?'); + if (search.isEmpty && queryStart == -1) return null; + final route = queryStart == -1 ? hash : hash.substring(0, queryStart); + return '$origin$pathname$route'; +} - if (hash.isNotEmpty) { - final queryIndex = hash.indexOf('?'); - if (queryIndex != -1) { - return Uri.splitQueryString(hash.substring(queryIndex + 1)); - } +/// A captured sign-in callback, and whether it was still in the URL because +/// the `web/index.html` script is missing or did not run. +typedef CapturedCallback = ({CallbackParams params, bool scriptMissing}); + +/// Captures the sign-in callback from the query `web/index.html` stashed +/// ([stashedQuery]), falling back to the current URL's [hash]. +CapturedCallback captureCallback({ + required String? stashedQuery, + required String hash, +}) { + if (stashedQuery != null) { + return ( + params: _parseQuery(stashedQuery), + scriptMissing: false, + ); } + final query = callbackQueryFromHash(hash); + if (query == null) { + return (params: const NoCallbackParams(), scriptMissing: false); + } + return ( + params: _parseQuery(query), + scriptMissing: true, + ); +} - return {}; +/// Parses a callback [query], or returns [WebCallbackMalformed] when its +/// percent encoding or UTF-8 is malformed, so a crafted link cannot stop the +/// app from starting. +CallbackParams _parseQuery(String query) { + try { + return parseCallbackParams(Uri.splitQueryString(query)); + } catch (e, st) { + if (e is! ArgumentError && e is! FormatException) rethrow; + // `describeFailure` drops the input a failure carries, per the logging + // rule in CLAUDE.md. + _logger.warning( + 'Ignoring a sign-in callback with a malformed query', + attributes: {'failure': describeFailure(e)}, + stackTrace: st, + ); + return const WebCallbackMalformed(); + } } int? _parseIntOrNull(String? value) { diff --git a/lib/src/modules/auth/platform/callback_service.dart b/lib/src/modules/auth/platform/callback_service.dart index e767cc4af..99b249b5b 100644 --- a/lib/src/modules/auth/platform/callback_service.dart +++ b/lib/src/modules/auth/platform/callback_service.dart @@ -11,13 +11,15 @@ export 'callback_params.dart'; abstract final class CallbackParamsCapture { /// Capture callback params from current URL. /// - /// On web, extracts tokens from URL query params. + /// On web, reads the tokens `web/index.html` moved out of the URL, or the + /// URL itself when that script is missing. /// On native, returns [NoCallbackParams]. static CallbackParams captureNow() => impl.captureCallbackParamsNow(); } /// Clears OAuth callback parameters from the browser URL. /// -/// On web, removes tokens from the URL and browser history. +/// On web, removes every query from the page URL on load, in the address bar +/// and in the hash route, including the sign-in callback's tokens. /// On native, this is a no-op. void clearCallbackUrl() => impl.clearCallbackUrl(); diff --git a/lib/src/modules/auth/platform/callback_service_web.dart b/lib/src/modules/auth/platform/callback_service_web.dart index d480e4ce7..4db17de4a 100644 --- a/lib/src/modules/auth/platform/callback_service_web.dart +++ b/lib/src/modules/auth/platform/callback_service_web.dart @@ -1,32 +1,46 @@ import 'dart:js_interop'; +import 'dart:js_interop_unsafe'; +import 'package:soliplex_logging/soliplex_logging.dart'; import 'package:web/web.dart' as web; import 'callback_params.dart'; import 'callback_params_parser.dart'; -/// Captures callback params from current URL. +final Logger _logger = LogManager.instance.getLogger('soliplex.auth_callback'); + +/// Where `web/index.html` leaves the callback query it moved out of the URL. +const _stashKey = 'soliplexCallbackQuery'; + +/// Captures the sign-in callback, from the `web/index.html` stash or, when the +/// script is missing, from the URL. CallbackParams captureCallbackParamsNow() { - final params = extractQueryParams( - search: web.window.location.search, + final key = _stashKey.toJS; + final stashed = globalContext.getProperty<JSString?>(key)?.toDart; + globalContext.delete(key); + final captured = captureCallback( + stashedQuery: stashed, hash: web.window.location.hash, ); - return parseCallbackParams(params); + if (captured.scriptMissing) { + _logger.error( + 'Sign-in callback reached the app still in the URL: web/index.html ' + 'lacks the callback script (see docs/authoring-a-flavor.md)', + ); + } + return captured.params; } -/// Clears OAuth callback parameters from the browser URL. +/// Removes every query from the page URL on load, in the address bar and in +/// the hash route, including the sign-in callback's tokens. void clearCallbackUrl() { - final origin = web.window.location.origin; - final pathname = web.window.location.pathname; - var hash = web.window.location.hash; - - if (hash.isNotEmpty) { - final queryIndex = hash.indexOf('?'); - if (queryIndex != -1) { - hash = hash.substring(0, queryIndex); - } - } - - final cleanUrl = '$origin$pathname$hash'; - web.window.history.replaceState(JSObject(), '', cleanUrl); + final location = web.window.location; + final url = urlWithoutQueries( + origin: location.origin, + pathname: location.pathname, + search: location.search, + hash: location.hash, + ); + if (url == null) return; + web.window.history.replaceState(JSObject(), '', url); } diff --git a/lib/src/modules/auth/ui/auth_callback_screen.dart b/lib/src/modules/auth/ui/auth_callback_screen.dart index 6dd4a5b29..fa894c4c7 100644 --- a/lib/src/modules/auth/ui/auth_callback_screen.dart +++ b/lib/src/modules/auth/ui/auth_callback_screen.dart @@ -54,6 +54,9 @@ class _AuthCallbackScreenState extends ConsumerState<AuthCallbackScreen> { case NoCallbackParams(): _fail('No callback parameters received.'); return; + case WebCallbackMalformed(): + _fail('The sign-in response could not be read. Please try again.'); + return; case WebCallbackError(:final error): _logger.warning( 'Auth callback returned an OAuth error', diff --git a/lib/src/modules/auth/ui/home_screen.dart b/lib/src/modules/auth/ui/home_screen.dart index b70165ebe..5499bbd08 100644 --- a/lib/src/modules/auth/ui/home_screen.dart +++ b/lib/src/modules/auth/ui/home_screen.dart @@ -4,6 +4,7 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; import 'package:signals_flutter/signals_flutter.dart'; import 'package:soliplex_agent/soliplex_agent.dart' hide AuthException; +import 'package:soliplex_logging/soliplex_logging.dart'; import '../../../core/routes.dart'; import '../../../shared/markdown/prose_markdown.dart'; @@ -22,6 +23,8 @@ import '../../../shared/selectable_content.dart'; import '../../../shared/type_to_focus.dart'; import 'package:soliplex_design/soliplex_design.dart'; +final Logger _logger = LogManager.instance.getLogger('soliplex.home_screen'); + class HomeScreen extends ConsumerStatefulWidget { const HomeScreen({ super.key, @@ -111,12 +114,18 @@ class _HomeScreenState extends ConsumerState<HomeScreen> { }); final autoConnect = widget.autoConnectUrl; - if (autoConnect != null) { - _urlController.text = autoConnect; + final knownServer = + autoConnect == null ? null : _knownServerAt(autoConnect); + if (knownServer != null) { + _urlController.text = knownServer.serverUrl.toString(); WidgetsBinding.instance.addPostFrameCallback((_) { if (mounted) _connect(); }); } else { + if (autoConnect != null) { + _logger.warning( + 'Ignored an auto-connect address that names no saved server'); + } final defaultUrl = widget.defaultBackendUrl; if (defaultUrl != null && widget.serverManager.servers.value.isEmpty) { _urlController.text = defaultUrl; @@ -124,6 +133,16 @@ class _HomeScreenState extends ConsumerState<HomeScreen> { } } + // The route's address can come from outside the app (an iOS deep link, or + // a same-tab hash change on web), so only a server the user already added + // is connected, and at the address it was added with. + ServerEntry? _knownServerAt(String address) { + final uri = Uri.tryParse(address); + if (uri == null || !uri.hasAuthority || uri.host.isEmpty) return null; + if (uri.scheme != 'http' && uri.scheme != 'https') return null; + return widget.serverManager.servers.value[serverIdFromUrl(uri)]; + } + void _onUrlChanged() { final hasText = _urlController.text.isNotEmpty; if (hasText != _hasUrlText) { diff --git a/test/modules/auth/platform/callback_params_parser_test.dart b/test/modules/auth/platform/callback_params_parser_test.dart index 03cdf05f7..b80e4b3c9 100644 --- a/test/modules/auth/platform/callback_params_parser_test.dart +++ b/test/modules/auth/platform/callback_params_parser_test.dart @@ -1,6 +1,20 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:soliplex_frontend/src/modules/auth/platform/callback_params.dart'; import 'package:soliplex_frontend/src/modules/auth/platform/callback_params_parser.dart'; +import 'package:soliplex_logging/soliplex_logging.dart'; + +MemorySink _captureLogs() { + final sink = MemorySink(); + LogManager.instance.addSink(sink); + addTearDown(() => LogManager.instance.removeSink(sink)); + return sink; +} + +/// The single warning `soliplex.auth_callback` recorded. +LogRecord _warning(MemorySink sink) => sink.records + .where((r) => + r.loggerName == 'soliplex.auth_callback' && r.level == LogLevel.warning) + .single; void main() { group('parseCallbackParams', () { @@ -35,22 +49,6 @@ void main() { expect((result as WebCallbackSuccess).accessToken, 'abc123'); }); - test('access_token param used as fallback', () { - final result = parseCallbackParams({'access_token': 'xyz789'}); - - expect(result, isA<WebCallbackSuccess>()); - expect((result as WebCallbackSuccess).accessToken, 'xyz789'); - }); - - test('token takes precedence over access_token', () { - final result = parseCallbackParams({ - 'token': 'primary', - 'access_token': 'fallback', - }); - - expect((result as WebCallbackSuccess).accessToken, 'primary'); - }); - test('refresh_token and expires_in forwarded when present', () { final result = parseCallbackParams({ 'token': 'abc', @@ -98,46 +96,144 @@ void main() { }); }); - group('extractQueryParams', () { - test('parses from search string', () { - final result = extractQueryParams( - search: '?code=abc&state=xyz', - hash: '', + group('callbackQueryFromHash', () { + test('returns the query of the sign-in callback route', () { + expect(callbackQueryFromHash('#/auth/callback?token=abc&expires_in=60'), + 'token=abc&expires_in=60'); + }); + + test('ignores other routes, even with a query', () { + expect(callbackQueryFromHash('#/lobby?server=x'), isNull); + expect(callbackQueryFromHash('#/?url=https%3A%2F%2Fa&returnTo=%2Fr'), + isNull); + expect(callbackQueryFromHash('#/auth/callback'), isNull); + expect(callbackQueryFromHash(''), isNull); + }); + }); + + group('urlWithoutQueries', () { + const page = 'https://soliplex.example/app/'; + + String? clear(String hash, + {String search = '', String pathname = '/app/'}) => + urlWithoutQueries( + origin: 'https://soliplex.example', + pathname: pathname, + search: search, + hash: hash, + ); + + test("drops a sign-in callback's tokens", () { + expect(clear('#/auth/callback?token=x'), '$page#/auth/callback'); + }); + + test("drops a lobby route's query", () { + expect(clear('#/lobby?server=x'), '$page#/lobby'); + }); + + test("drops the home route's query", () { + expect(clear('#/?url=x'), '$page#/'); + }); + + test('drops the query of a hash with no path', () { + expect(clear('#?url=x'), '$page#'); + }); + + test('drops the query of another spelling of the home route', () { + expect(clear('#//?url=x'), '$page#//'); + }); + + test('drops tokens in the address bar query', () { + expect(clear('#/', search: '?token=a&refresh_token=b'), '$page#/'); + }); + + test('drops the address bar query when there is no hash', () { + expect(clear('', search: '?token=a'), page); + }); + + test('leaves a URL with no query unchanged', () { + expect(clear('#/lobby'), isNull); + }); + + test('keeps the origin when the path reads as another host', () { + expect(clear('#/auth/callback?token=abc', pathname: '//evil.example/'), + startsWith('https://soliplex.example/')); + }); + }); + + group('captureCallback', () { + test('reads the query index.html stashed', () { + final captured = captureCallback( + stashedQuery: 'token=abc&id_token=id', + hash: '#/auth/callback', ); - expect(result, {'code': 'abc', 'state': 'xyz'}); + expect(captured.scriptMissing, isFalse); + final success = captured.params as WebCallbackSuccess; + expect(success.accessToken, 'abc'); + expect(success.idToken, 'id'); }); - test('empty search falls back to hash query', () { - final result = extractQueryParams( - search: '', - hash: '#/callback?token=abc&expires_in=3600', + test('falls back to the URL and flags the missing script', () { + final captured = captureCallback( + stashedQuery: null, + hash: '#/auth/callback?token=abc', ); - expect(result, {'token': 'abc', 'expires_in': '3600'}); + expect(captured.scriptMissing, isTrue); + expect((captured.params as WebCallbackSuccess).accessToken, 'abc'); }); - test('both empty returns empty map', () { - final result = extractQueryParams(search: '', hash: ''); - expect(result, isEmpty); + test('an ordinary page load is no callback and no missing script', () { + final captured = + captureCallback(stashedQuery: null, hash: '#/lobby?server=x'); + + expect(captured.params, isA<NoCallbackParams>()); + expect(captured.scriptMissing, isFalse); }); - test('hash without query portion returns empty map', () { - final result = extractQueryParams( - search: '', - hash: '#/callback', + test('ignores an access_token key', () { + final captured = captureCallback( + stashedQuery: 'access_token=abc', + hash: '#/auth/callback', ); - expect(result, isEmpty); + expect(captured.params, isA<NoCallbackParams>()); }); - test('search takes precedence over hash', () { - final result = extractQueryParams( - search: '?from=search', - hash: '#/path?from=hash', + test('an illegal percent encoding in the stash is malformed', () { + final sink = _captureLogs(); + + final captured = captureCallback( + stashedQuery: 'token=secret%ZZ', + hash: '#/auth/callback', + ); + + expect(captured.params, isA<WebCallbackMalformed>()); + expect(captured.scriptMissing, isFalse); + final record = _warning(sink); + expect(record.attributes, {'failure': 'ArgumentError'}); + expect(record.error, isNull); + expect(record.stackTrace, isNotNull); + expect(record.toString(), isNot(contains('secret'))); + }); + + test('invalid UTF-8 in the URL is malformed and still flags the script', + () { + final sink = _captureLogs(); + + final captured = captureCallback( + stashedQuery: null, + hash: '#/auth/callback?token=secret%E0%A4', ); - expect(result['from'], 'search'); + expect(captured.params, isA<WebCallbackMalformed>()); + expect(captured.scriptMissing, isTrue); + final record = _warning(sink); + expect(record.attributes['failure'], startsWith('FormatException')); + expect(record.error, isNull); + expect(record.toString(), isNot(contains('secret'))); + expect(record.attributes.toString(), isNot(contains('secret'))); }); }); } diff --git a/test/modules/auth/ui/auth_callback_screen_test.dart b/test/modules/auth/ui/auth_callback_screen_test.dart index acbf7046c..d5dd6a670 100644 --- a/test/modules/auth/ui/auth_callback_screen_test.dart +++ b/test/modules/auth/ui/auth_callback_screen_test.dart @@ -110,6 +110,22 @@ void main() { expect(find.text('Back to home'), findsOneWidget); }); + testWidgets('shows error when the callback query could not be read', + (tester) async { + final serverManager = _createServerManager(); + await tester.pumpWidget(_buildApp( + serverManager: serverManager, + callbackParams: const WebCallbackMalformed(), + )); + await tester.pumpAndSettle(); + + expect( + find.text('The sign-in response could not be read. ' + 'Please try again.'), + findsOneWidget); + expect(find.text('Back to home'), findsOneWidget); + }); + testWidgets('shows error when callback has error', (tester) async { final serverManager = _createServerManager(); await tester.pumpWidget(_buildApp( diff --git a/test/modules/auth/ui/home_screen_test.dart b/test/modules/auth/ui/home_screen_test.dart index e8a5272be..df9a3ed48 100644 --- a/test/modules/auth/ui/home_screen_test.dart +++ b/test/modules/auth/ui/home_screen_test.dart @@ -23,6 +23,7 @@ import 'package:soliplex_frontend/src/modules/auth/ui/server_sign_out_control.da import 'package:soliplex_frontend/src/modules/auth/ui/server_status_dot.dart'; import 'package:soliplex_frontend/src/shared/markdown/prose_markdown.dart'; import 'package:soliplex_frontend/version.dart'; +import 'package:soliplex_logging/soliplex_logging.dart'; import '../../../helpers/fakes.dart'; @@ -977,38 +978,133 @@ void main() { expect(field.controller!.text, isEmpty); }); - testWidgets('autoConnectUrl sets URL and triggers connect', (tester) async { + for (final address in [ + 'https://other.example', + 'https://known.example.com:@evil.example.com', + 'known.example.com:@evil.example.com', + 'known.example.com@evil.example.com', + ' https://known.example.com:@evil.example.com', + ]) { + testWidgets('autoConnectUrl for an unknown "$address" is ignored', + (tester) async { + final serverManager = _createServerManager(); + serverManager.addServer( + serverId: 'https://known.example.com', + serverUrl: Uri.parse('https://known.example.com'), + ); + final encodedUrl = Uri.encodeComponent(address); + + await tester.pumpWidget(_buildApp( + serverManager: serverManager, + discover: _noAuthDiscover, + initialLocation: '/?url=$encodedUrl', + )); + await tester.pumpAndSettle(); + + expect(serverManager.servers.value.keys, ['https://known.example.com']); + expect(find.text('Lobby placeholder'), findsNothing); + final field = tester.widget<TextFormField>(find.byType(TextFormField)); + expect(field.controller!.text, isEmpty); + }); + } + + testWidgets('an ignored autoConnectUrl is recorded without the address', + (tester) async { + final sink = MemorySink(); + LogManager.instance.addSink(sink); + addTearDown(() => LogManager.instance.removeSink(sink)); final serverManager = _createServerManager(); - final encodedUrl = Uri.encodeComponent('https://demo.example.com'); + serverManager.addServer( + serverId: 'https://known.example.com', + serverUrl: Uri.parse('https://known.example.com'), + ); + final encodedUrl = Uri.encodeComponent('https://other.example/secret'); await tester.pumpWidget(_buildApp( serverManager: serverManager, + initialLocation: '/?url=$encodedUrl', + )); + await tester.pumpAndSettle(); + + final record = sink.records + .where((r) => + r.loggerName == 'soliplex.home_screen' && + r.level == LogLevel.warning) + .single; + expect(record.message, + 'Ignored an auto-connect address that names no saved server'); + expect(record.toString(), isNot(contains('other.example'))); + expect(record.toString(), isNot(contains('secret'))); + }); + + testWidgets( + 'an unknown autoConnectUrl with no saved servers leaves the default ' + 'URL, unconnected', (tester) async { + final sink = MemorySink(); + LogManager.instance.addSink(sink); + addTearDown(() => LogManager.instance.removeSink(sink)); + final encodedUrl = Uri.encodeComponent('https://other.example/secret'); + + await tester.pumpWidget(_buildApp( + serverManager: _createServerManager(), discover: _noAuthDiscover, + defaultBackendUrl: 'https://default.example.com', initialLocation: '/?url=$encodedUrl', )); await tester.pumpAndSettle(); - // Should have auto-connected and navigated to lobby. - expect(find.text('Lobby placeholder'), findsOneWidget); + final field = tester.widget<TextFormField>(find.byType(TextFormField)); + expect(field.controller!.text, 'https://default.example.com'); + expect(find.text('Lobby placeholder'), findsNothing); + final record = sink.records + .where((r) => + r.loggerName == 'soliplex.home_screen' && + r.level == LogLevel.warning) + .single; + expect(record.message, + 'Ignored an auto-connect address that names no saved server'); + expect(record.toString(), isNot(contains('other.example'))); }); - testWidgets('autoConnectUrl takes precedence over defaultBackendUrl', + testWidgets('autoConnectUrl connects a known server at its stored address', (tester) async { final serverManager = _createServerManager(); - final encodedUrl = Uri.encodeComponent('https://auto.example.com'); + serverManager.addServer( + serverId: 'https://demo.example.com', + serverUrl: Uri.parse('https://demo.example.com'), + ); + final encodedUrl = + Uri.encodeComponent('https://demo.example.com/other/path'); await tester.pumpWidget(_buildApp( serverManager: serverManager, discover: _noAuthDiscover, - defaultBackendUrl: 'https://default.example.com', initialLocation: '/?url=$encodedUrl', )); + final field = tester.widget<TextFormField>(find.byType(TextFormField)); await tester.pumpAndSettle(); - // Should auto-connect to the autoConnectUrl, not defaultBackendUrl. - expect( - serverManager.servers.value.containsKey('https://auto.example.com'), - isTrue); + expect(field.controller!.text, 'https://demo.example.com'); + expect(find.text('Lobby placeholder'), findsOneWidget); + }); + + testWidgets('autoConnectUrl sets URL and triggers connect', (tester) async { + final serverManager = _createServerManager(); + serverManager.addServer( + serverId: 'https://demo.example.com', + serverUrl: Uri.parse('https://demo.example.com'), + ); + final encodedUrl = Uri.encodeComponent('https://demo.example.com'); + + await tester.pumpWidget(_buildApp( + serverManager: serverManager, + discover: _noAuthDiscover, + initialLocation: '/?url=$encodedUrl', + )); + await tester.pumpAndSettle(); + + // Should have auto-connected and navigated to lobby. + expect(find.text('Lobby placeholder'), findsOneWidget); }); testWidgets('re-fills URL field when last server is removed', diff --git a/web/index.html b/web/index.html index 7bab97f9b..58ef618f6 100644 --- a/web/index.html +++ b/web/index.html @@ -15,6 +15,28 @@ <base href="$FLUTTER_BASE_HREF"> <meta charset="UTF-8"> + <title>Soliplex + + @@ -27,7 +49,6 @@ - Soliplex