Conversation
- Login screen: add ref.listen(selectedServerProvider) to handle async provider loading race fixes homeserver URL staying as localhost when the provider hasn't loaded by the time initState's post-frame callback fires. Also loads saved_homeserver from prefs (was saved but never read). - Register screen: now reads from selectedServerProvider (was completely missing always showed hardcoded localhost defaults). - Both screens: derive AS URL from homeserver (same host, port 3000) when server has no explicit AS URL set. - Add integration test 3.6: full manual login flow from server selection through credential entry, verifying the homeserver field is populated from the selected server.
…condition The root cause: login/register screens initialized TextEditingControllers with hardcoded 'http://localhost:8008' defaults. The reactive ref.listen for selectedServerProvider could fire AFTER the user had typed a different URL, overwriting their input back to the selected server's (old) URL. Changes: - Controllers now start empty (no hardcoded defaults) - _tryApplyServerUrl() checks field.isEmpty before writing, so user input is never overwritten by late-firing provider updates - _submit() derives AS URL from homeserver at submit time if empty (same host, port 3000) users don't need to expand Advanced - _submit() validates homeserver is not empty before proceeding - Saved-credentials path also respects isEmpty guard
Root cause: Synapse serves /.well-known/matrix/client with public_baseurl (http://localhost:8008 in Docker by default). The Matrix SDK's checkHomeserver() follows well-known and overwrites client.homeserver to localhost, so all subsequent API calls go to the wrong server. Client-side fix: - Pass checkWellKnown: false to checkHomeserver() when the user explicitly enters a URL, well-known discovery must not override it - Add 10-second timeout on checkHomeserver() so unreachable servers fail fast instead of hanging for minutes Server-side fix: - homeserver.yaml.template: use MATRIX_SERVER_NAME and MATRIX_PUBLIC_BASEURL env vars instead of hardcoded localhost values - docker-compose: pass these env vars to the Synapse container - .env.example: document MATRIX_PUBLIC_BASEURL for remote deployments Tests: - Update mocktail stubs to match checkWellKnown named parameter
There was a problem hiding this comment.
Pull request overview
This PR advances the Matrix migration by parameterizing Synapse’s server identity/public URL for Docker deployments and updating the Flutter auth flows to better integrate server selection while preventing .well-known from overriding user-entered homeserver URLs.
Changes:
- Make Synapse
server_name/public_baseurlconfigurable via env vars (template + docker-compose +.env.example). - Update login/register UI to prefill from selected server and derive AS URL when Advanced is not used.
- Update Matrix client homeserver checks to disable
.well-knownoverride and add a connection timeout; adjust unit/integration tests accordingly.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
synapse/homeserver.yaml.template |
Replace hardcoded localhost values with ${MATRIX_SERVER_NAME} / ${MATRIX_PUBLIC_BASEURL}. |
synapse/homeserver.yaml |
Clarify that the template is used for Docker; keep local-dev config. |
docker-compose.yml |
Provide MATRIX_SERVER_NAME / MATRIX_PUBLIC_BASEURL env vars to Synapse service. |
.env.example |
Document and add MATRIX_PUBLIC_BASEURL. |
app/lib/features/auth/providers/auth_provider.dart |
Disable .well-known override in checkHomeserver and add a timeout. |
app/lib/features/auth/screens/login_screen.dart |
Prefill homeserver/AS URL from selected server or saved prefs; derive AS URL on submit. |
app/lib/features/auth/screens/register_screen.dart |
Prefill homeserver/AS URL from selected server; derive AS URL on submit. |
app/test/features/auth/providers/auth_provider_test.dart |
Update mocks for checkHomeserver(..., checkWellKnown: ...) signature. |
app/integration_test/visual_test.dart |
Add a manual login flow test covering server selection → login. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| void _deriveAsUrl(String homeserverUrl, [String explicitAsUrl = '']) { | ||
| if (_asUrlController.text.isNotEmpty) return; | ||
| if (explicitAsUrl.isNotEmpty) { | ||
| _asUrlController.text = explicitAsUrl; | ||
| } else { | ||
| try { | ||
| final uri = Uri.parse(homeserverUrl); | ||
| _asUrlController.text = '${uri.scheme}://${uri.host}:3000'; | ||
| } catch (_) {} |
There was a problem hiding this comment.
AS URL derivation uses Uri.parse(homeserverUrl) and then ${uri.scheme}://${uri.host}:3000. For inputs like localhost:8008 or matrix.example.com (no scheme), Dart parses these with an empty host, producing invalid URLs (e.g., localhost://:3000) without throwing. Validate that the parsed URI is absolute (hasScheme and non-empty host) and show an error / add a default scheme (e.g., https) instead of silently setting an invalid AS URL.
| // Ensure AS URL is set (derive from homeserver if user never expanded Advanced) | ||
| var asUrl = _asUrlController.text.trim(); | ||
| if (asUrl.isEmpty) { | ||
| try { | ||
| final uri = Uri.parse(homeserver); | ||
| asUrl = '${uri.scheme}://${uri.host}:3000'; | ||
| _asUrlController.text = asUrl; | ||
| } catch (_) {} | ||
| } |
There was a problem hiding this comment.
In _submit, AS URL is auto-derived with Uri.parse(homeserver) and ${uri.scheme}://${uri.host}:3000, but Uri.parse often succeeds with an empty host for non-absolute inputs and you'll persist an invalid saved_as_url. Consider rejecting non-absolute homeserver URLs (or normalizing by prepending http:///https://) before deriving/persisting asUrl.
| } else { | ||
| try { | ||
| final uri = Uri.parse(server.homeserverUrl); | ||
| _asUrlController.text = '${uri.scheme}://${uri.host}:3000'; |
There was a problem hiding this comment.
_tryApplyServerUrl derives the AS URL via Uri.parse(server.homeserverUrl) and then ${uri.scheme}://${uri.host}:3000. Uri.parse will not throw for many malformed/non-absolute values and may yield an empty host, resulting in an invalid AS URL being populated. Please validate uri.hasScheme && uri.host.isNotEmpty (or normalize the homeserver URL first) before setting the controller.
| _asUrlController.text = '${uri.scheme}://${uri.host}:3000'; | |
| if (uri.hasScheme && uri.host.isNotEmpty) { | |
| _asUrlController.text = '${uri.scheme}://${uri.host}:3000'; | |
| } |
| // Ensure AS URL is set (derive from homeserver if user never expanded Advanced) | ||
| var asUrl = _asUrlController.text.trim(); | ||
| if (asUrl.isEmpty) { | ||
| try { | ||
| final uri = Uri.parse(homeserver); | ||
| asUrl = '${uri.scheme}://${uri.host}:3000'; | ||
| _asUrlController.text = asUrl; | ||
| } catch (_) {} | ||
| } |
There was a problem hiding this comment.
In _submit, AS URL derivation uses Uri.parse(homeserver) and ${uri.scheme}://${uri.host}:3000. Because Uri.parse may succeed with an empty host for inputs without a scheme, this can silently set an invalid AS URL. Validate/normalize the homeserver URL (absolute URI with scheme+host) before deriving and passing asBaseUrl.
| // checkWellKnown: false — the user explicitly entered the URL, so we | ||
| // must NOT let the server's /.well-known override it (Synapse returns | ||
| // public_baseurl which is often http://localhost:8008 in Docker setups). | ||
| await client | ||
| .checkHomeserver(Uri.parse(homeserverUrl), checkWellKnown: false) | ||
| .timeout( | ||
| const Duration(seconds: 10), | ||
| onTimeout: () => throw Exception( | ||
| 'Connection timeout – server not reachable', | ||
| ), | ||
| ); |
There was a problem hiding this comment.
checkHomeserver(Uri.parse(homeserverUrl), ...) will throw (or behave unexpectedly) if the user enters a non-absolute URL (missing scheme/host). Since the UI no longer pre-fills http://..., consider validating/normalizing homeserverUrl before Uri.parse (e.g., ensure scheme+host, and return a user-friendly error if invalid) rather than relying on the raw exception string.
| // checkWellKnown: false — same rationale as login(): the user typed | ||
| // the URL, don't let well-known override it. | ||
| await client | ||
| .checkHomeserver(Uri.parse(homeserverUrl), checkWellKnown: false) | ||
| .timeout( | ||
| const Duration(seconds: 10), | ||
| onTimeout: () => throw Exception( | ||
| 'Connection timeout – server not reachable', | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Same issue as login(): Uri.parse(homeserverUrl) assumes an absolute URL. If the user enters a value without scheme/host, this will fail and the error message will include a low-level exception. Add explicit validation/normalization of homeserverUrl before calling checkHomeserver so registration fails with a clear, actionable message.
| when(() => mockClient.checkHomeserver(any(), checkWellKnown: any(named: 'checkWellKnown'))) | ||
| .thenAnswer((_) async => (null, matrix.GetVersionsResponse(versions: ['v1.1']), <matrix.LoginFlow>[])); |
There was a problem hiding this comment.
The updated stubs accept any checkWellKnown value, which means the tests won’t catch regressions if production code stops passing checkWellKnown: false (the new intended behavior). Consider matching explicitly on checkWellKnown: false in these when(() => mockClient.checkHomeserver(...)) expectations.
No description provided.