mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(mobile): keep TLS on relays joined by invite (#3139)
## Summary Communities joined via an invite link never connect: the app dials `ws://` on port 80 instead of `wss://` on 443 and sits on "Reconnecting…" indefinitely. `RelayConfig.baseUrl` is documented as an HTTP origin, but the two onboarding flows disagree on what they persist: - **Device pairing** validates and stores `https://` — `pairing_provider.dart:657` throws on anything else. - **Invite join** stores the relay URL straight off the invite link, and `deep_link.dart:165` always emits `ws://` or `wss://`. `wsUrl` only special-cased `https://`, so a `wss://` base fell through to the plaintext branch: ```dart final scheme = uri.scheme == 'https' ? 'wss' : 'ws'; // 'wss' is not 'https' ``` The claim request itself succeeds, because `_claimUrlFromRelay` (`invite_join_provider.dart:242`) maps `wss → https` explicitly. Only the socket path is missing that conversion — which is why the community appears, correctly named, and then never loads. The same `baseUrl` also feeds `/query` (`relay_session.dart:136`), media upload (`media_upload.dart:765`), Blossom auth (`media_auth.dart:128`) and `relayClientProvider` (`relay_provider.dart:113`), so those requests were malformed too. Where port 80 *does* answer, it is additionally a silent TLS downgrade after `validateInviteRelayUri` insisted on `wss://`. This folds the websocket schemes back to their HTTP equivalents in `baseUrl` itself, so every consumer is correct by construction rather than needing a second getter remembered at each call site, and communities **already persisted** with `wss://` are repaired on read without a migration. `community_icon_provider.dart:46` already performs this same conversion locally. One subtlety worth flagging for review: the normalization is derived in the getter rather than applied in the constructor, so the constructor stays `const`. The compile-time fallback at `relay_provider.dart:77` relies on const canonicalization for a stable identity across rebuilds, and Riverpod's `defaultUpdateShouldNotify` is `previous != next` (`element.dart:361`), which falls back to identity for this class. A `factory` constructor here yields a fresh instance per rebuild, which tears down and resubscribes every listener — `channels_provider_test.dart` catches it as an unexpected unsubscribe during reconnect. ### Related issue Fixes #2662. ### Testing `flutter test` — **705 passed, 1 skipped, 0 failed** `flutter analyze` — No issues found `dart format --set-exit-if-changed .` — 249 files, 0 changed Run against the Hermit-pinned SDK (Flutter 3.41.7 / Dart 3.11.5), matching CI. 10 new unit tests in `mobile/test/shared/relay/relay_config_test.dart` covering both onboarding schemes, `http`/`https` passthrough, non-default ports, and agreement between the invite and pairing paths for the same relay. Verified end-to-end against a self-hosted relay behind `tailscale serve`, which terminates TLS on 443 and leaves port 80 closed. Relay logs show the invite claim succeeding over HTTPS at the moment of joining, while no WebSocket connection ever arrives — no `WebSocket connection established`, no NIP-42 auth, no `kind:0` profile, no push registration — across the relay's entire history, even though the member row is present and correct. Port-80 refusals are not logged by `tailscaled`'s netstack, which is why the retries leave no trace server-side. Reproduced on both iOS and Android. --------- Signed-off-by: Krishna C <github@kumb.uk>
This commit is contained in:
@@ -79,7 +79,13 @@ class ComposeDraftsNotifier extends Notifier<List<ComposeDraft>> {
|
||||
_prefsKey = '$_draftsPrefsKey:${config.baseUrl}:$pubkey';
|
||||
|
||||
final prefs = ref.read(savedPrefsProvider);
|
||||
final raw = prefs.getString(_prefsKey);
|
||||
final raw = readMigratedPref<String>(
|
||||
prefs,
|
||||
canonicalKey: _prefsKey,
|
||||
legacyKey: '$_draftsPrefsKey:${config.storedOrigin}:$pubkey',
|
||||
read: prefs.getString,
|
||||
write: prefs.setString,
|
||||
);
|
||||
if (raw == null) return const [];
|
||||
try {
|
||||
final decoded = jsonDecode(raw);
|
||||
|
||||
@@ -19,8 +19,16 @@ class RecentSearchesNotifier extends Notifier<List<String>> {
|
||||
final pubkey = ref.watch(myPubkeyProvider) ?? 'anon';
|
||||
_prefsKey = '$_recentSearchesPrefsKey:${config.baseUrl}:$pubkey';
|
||||
|
||||
final prefs = ref.read(savedPrefsProvider);
|
||||
final stored =
|
||||
ref.read(savedPrefsProvider).getStringList(_prefsKey) ?? const [];
|
||||
readMigratedPref<List<String>>(
|
||||
prefs,
|
||||
canonicalKey: _prefsKey,
|
||||
legacyKey: '$_recentSearchesPrefsKey:${config.storedOrigin}:$pubkey',
|
||||
read: prefs.getStringList,
|
||||
write: prefs.setStringList,
|
||||
) ??
|
||||
const [];
|
||||
return List.unmodifiable(
|
||||
stored
|
||||
.map((query) => query.trim())
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
import 'dart:async';
|
||||
|
||||
import 'package:shared_preferences/shared_preferences.dart';
|
||||
|
||||
/// Reads an identity-scoped preference, migrating values left under a
|
||||
/// pre-canonicalization relay origin.
|
||||
///
|
||||
/// Identity-scoped keys embed the relay origin, and that origin used to be
|
||||
/// whatever scheme the onboarding flow happened to persist — `wss://` for an
|
||||
/// invite join, `https://` for device pairing. Now that [RelayConfig.baseUrl]
|
||||
/// canonicalizes the scheme, an install that joined by invite would compute a
|
||||
/// different key than the one its data was written under, leaving that data on
|
||||
/// disk but unreachable.
|
||||
///
|
||||
/// The value under [canonicalKey] wins. Otherwise a value under [legacyKey] is
|
||||
/// returned straight away and promoted onto the canonical key in the
|
||||
/// background. The legacy entry is removed only once the copy reports success,
|
||||
/// so a migration interrupted mid-flight leaves the original readable and the
|
||||
/// next read simply retries.
|
||||
///
|
||||
/// Pass matching accessors for the stored type — `getString`/`setString`, or
|
||||
/// `getStringList`/`setStringList`.
|
||||
T? readMigratedPref<T>(
|
||||
SharedPreferences prefs, {
|
||||
required String canonicalKey,
|
||||
required String legacyKey,
|
||||
required T? Function(String key) read,
|
||||
required Future<bool> Function(String key, T value) write,
|
||||
}) {
|
||||
final canonical = read(canonicalKey);
|
||||
if (canonical != null) return canonical;
|
||||
|
||||
// Pairing-created communities already store an HTTP origin, so the two keys
|
||||
// coincide and there is nothing to migrate.
|
||||
if (canonicalKey == legacyKey) return null;
|
||||
|
||||
final legacy = read(legacyKey);
|
||||
if (legacy == null) return null;
|
||||
|
||||
unawaited(_promote(prefs, legacyKey, write(canonicalKey, legacy)));
|
||||
return legacy;
|
||||
}
|
||||
|
||||
Future<void> _promote(
|
||||
SharedPreferences prefs,
|
||||
String legacyKey,
|
||||
Future<bool> copy,
|
||||
) async {
|
||||
if (await copy) await prefs.remove(legacyKey);
|
||||
}
|
||||
@@ -1,4 +1,5 @@
|
||||
export 'app_lifecycle_provider.dart';
|
||||
export 'identity_scoped_prefs.dart';
|
||||
export 'media_auth.dart';
|
||||
export 'media_image.dart';
|
||||
export 'media_upload.dart';
|
||||
|
||||
@@ -10,12 +10,46 @@ import 'relay_client.dart';
|
||||
/// - `baseUrl` — where the relay lives (used for WS + media upload)
|
||||
/// - `nsec` — the user's signing key (drives NIP-42 AUTH and event sigs)
|
||||
class RelayConfig {
|
||||
final String baseUrl;
|
||||
const RelayConfig({required String baseUrl, this.nsec}) : _baseUrl = baseUrl;
|
||||
|
||||
/// Relay origin exactly as the active community stored it.
|
||||
final String _baseUrl;
|
||||
|
||||
/// Nostr secret key (bech32 nsec) for signing events and NIP-42 AUTH.
|
||||
final String? nsec;
|
||||
|
||||
const RelayConfig({required this.baseUrl, this.nsec});
|
||||
/// The origin as persisted, before scheme canonicalization.
|
||||
///
|
||||
/// Exists solely so identity-scoped storage keys written before [baseUrl]
|
||||
/// was canonicalized stay reachable — see [readMigratedPref]. Never use it
|
||||
/// for network I/O; [baseUrl] and [wsUrl] are the addresses to connect to.
|
||||
String get storedOrigin => _baseUrl;
|
||||
|
||||
/// Relay origin as an HTTP(S) URL.
|
||||
///
|
||||
/// Communities are persisted with whichever scheme their onboarding flow
|
||||
/// used: device pairing stores `https://` (it rejects anything else), while
|
||||
/// an invite join stores the `wss://` relay URL carried by the invite link.
|
||||
/// Every consumer treats this as an HTTP origin — [wsUrl], the `/query`
|
||||
/// endpoint, media upload and Blossom auth — so a `wss://` base silently
|
||||
/// degrades all of them. Folding the websocket schemes back here keeps both
|
||||
/// onboarding paths equivalent, including for already-persisted communities.
|
||||
///
|
||||
/// Derived rather than normalized in the constructor so that the constructor
|
||||
/// stays `const`: the compile-time fallback below relies on canonicalization
|
||||
/// to keep its identity stable across rebuilds, and Riverpod's default
|
||||
/// `updateShouldNotify` is `previous != next`, which falls back to identity
|
||||
/// here. A fresh instance per rebuild would resubscribe every listener.
|
||||
String get baseUrl {
|
||||
final uri = Uri.tryParse(_baseUrl);
|
||||
if (uri == null) return _baseUrl;
|
||||
final scheme = switch (uri.scheme) {
|
||||
'wss' => 'https',
|
||||
'ws' => 'http',
|
||||
_ => null,
|
||||
};
|
||||
return scheme == null ? _baseUrl : uri.replace(scheme: scheme).toString();
|
||||
}
|
||||
|
||||
/// Derive the websocket URL from the HTTP base URL.
|
||||
String get wsUrl {
|
||||
|
||||
@@ -34,6 +34,35 @@ void main() {
|
||||
return container;
|
||||
}
|
||||
|
||||
test(
|
||||
'an invite-joined community keeps its drafts after origin canonicalization',
|
||||
() async {
|
||||
// Written by a build that stored the invite link's wss:// origin
|
||||
// verbatim; RelayConfig now canonicalizes that to https://, so the key
|
||||
// the app computes no longer matches the key on disk.
|
||||
const legacyKey = 'compose_drafts_v1:wss://relay-a.example:pk_a';
|
||||
const canonicalKey = 'compose_drafts_v1:https://relay-a.example:pk_a';
|
||||
SharedPreferences.setMockInitialValues({
|
||||
legacyKey:
|
||||
'[{"key":"ch1","channel_id":"ch1","text":"unsent work",'
|
||||
'"updated_at":1700000000}]',
|
||||
});
|
||||
|
||||
final container = await containerWithPrefs(
|
||||
relayUrl: 'wss://relay-a.example',
|
||||
);
|
||||
|
||||
final drafts = container.read(composeDraftsProvider);
|
||||
expect(drafts, hasLength(1), reason: 'draft survives the upgrade');
|
||||
expect(drafts.single.text, 'unsent work');
|
||||
|
||||
await Future<void>.delayed(Duration.zero);
|
||||
final prefs = await SharedPreferences.getInstance();
|
||||
expect(prefs.getString(canonicalKey), isNotNull);
|
||||
expect(prefs.getString(legacyKey), isNull);
|
||||
},
|
||||
);
|
||||
|
||||
test('composeDraftKey separates channel and thread composers', () {
|
||||
expect(composeDraftKey('ch1'), 'ch1');
|
||||
expect(composeDraftKey('ch1', threadHeadId: 't1'), 'ch1:t1');
|
||||
|
||||
@@ -35,6 +35,27 @@ void main() {
|
||||
return container;
|
||||
}
|
||||
|
||||
test('an invite-joined community keeps its recent searches after '
|
||||
'origin canonicalization', () async {
|
||||
const legacyKey = 'recent_searches_v1:wss://relay-a.example:pk-a';
|
||||
const canonicalKey = 'recent_searches_v1:https://relay-a.example:pk-a';
|
||||
SharedPreferences.setMockInitialValues({
|
||||
legacyKey: <String>['nostr', 'relays'],
|
||||
});
|
||||
|
||||
final container = await containerWithPrefs(
|
||||
relayUrl: 'wss://relay-a.example',
|
||||
pubkey: 'pk-a',
|
||||
);
|
||||
|
||||
expect(container.read(recentSearchesProvider), ['nostr', 'relays']);
|
||||
|
||||
await Future<void>.delayed(Duration.zero);
|
||||
final prefs = await SharedPreferences.getInstance();
|
||||
expect(prefs.getStringList(canonicalKey), ['nostr', 'relays']);
|
||||
expect(prefs.getStringList(legacyKey), isNull);
|
||||
});
|
||||
|
||||
test(
|
||||
'normalizes, deduplicates, caps, and persists submitted queries',
|
||||
() async {
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
import 'package:buzz/shared/relay/identity_scoped_prefs.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:shared_preferences/shared_preferences.dart';
|
||||
|
||||
void main() {
|
||||
TestWidgetsFlutterBinding.ensureInitialized();
|
||||
|
||||
const canonical = 'k_v1:https://relay.example:pk';
|
||||
const legacy = 'k_v1:wss://relay.example:pk';
|
||||
|
||||
Future<SharedPreferences> prefsWith(Map<String, Object> values) async {
|
||||
SharedPreferences.setMockInitialValues(values);
|
||||
return SharedPreferences.getInstance();
|
||||
}
|
||||
|
||||
String? readString(SharedPreferences prefs) => readMigratedPref<String>(
|
||||
prefs,
|
||||
canonicalKey: canonical,
|
||||
legacyKey: legacy,
|
||||
read: prefs.getString,
|
||||
write: prefs.setString,
|
||||
);
|
||||
|
||||
test('returns the canonical value when present', () async {
|
||||
final prefs = await prefsWith({canonical: 'new', legacy: 'old'});
|
||||
expect(readString(prefs), 'new');
|
||||
});
|
||||
|
||||
test('falls back to the legacy value and returns it immediately', () async {
|
||||
final prefs = await prefsWith({legacy: 'carried over'});
|
||||
expect(readString(prefs), 'carried over');
|
||||
});
|
||||
|
||||
test('promotes the legacy value onto the canonical key', () async {
|
||||
final prefs = await prefsWith({legacy: 'carried over'});
|
||||
readString(prefs);
|
||||
await Future<void>.delayed(Duration.zero);
|
||||
|
||||
expect(prefs.getString(canonical), 'carried over');
|
||||
expect(prefs.getString(legacy), isNull, reason: 'legacy entry is cleared');
|
||||
});
|
||||
|
||||
test('is idempotent across repeated reads', () async {
|
||||
final prefs = await prefsWith({legacy: 'carried over'});
|
||||
readString(prefs);
|
||||
await Future<void>.delayed(Duration.zero);
|
||||
expect(readString(prefs), 'carried over');
|
||||
await Future<void>.delayed(Duration.zero);
|
||||
expect(prefs.getString(canonical), 'carried over');
|
||||
});
|
||||
|
||||
test('returns null when neither key holds a value', () async {
|
||||
final prefs = await prefsWith({});
|
||||
expect(readString(prefs), isNull);
|
||||
});
|
||||
|
||||
test('does not touch storage when the keys coincide', () async {
|
||||
// A pairing-created community already stores an HTTP origin, so canonical
|
||||
// and legacy are the same string and there is nothing to migrate.
|
||||
SharedPreferences.setMockInitialValues({canonical: 'only'});
|
||||
final prefs = await SharedPreferences.getInstance();
|
||||
final value = readMigratedPref<String>(
|
||||
prefs,
|
||||
canonicalKey: canonical,
|
||||
legacyKey: canonical,
|
||||
read: prefs.getString,
|
||||
write: prefs.setString,
|
||||
);
|
||||
await Future<void>.delayed(Duration.zero);
|
||||
|
||||
expect(value, 'only');
|
||||
expect(prefs.getString(canonical), 'only');
|
||||
});
|
||||
|
||||
test('migrates string lists as well as strings', () async {
|
||||
final prefs = await prefsWith({
|
||||
legacy: <String>['alpha', 'beta'],
|
||||
});
|
||||
final value = readMigratedPref<List<String>>(
|
||||
prefs,
|
||||
canonicalKey: canonical,
|
||||
legacyKey: legacy,
|
||||
read: prefs.getStringList,
|
||||
write: prefs.setStringList,
|
||||
);
|
||||
await Future<void>.delayed(Duration.zero);
|
||||
|
||||
expect(value, ['alpha', 'beta']);
|
||||
expect(prefs.getStringList(canonical), ['alpha', 'beta']);
|
||||
expect(prefs.getStringList(legacy), isNull);
|
||||
});
|
||||
}
|
||||
@@ -0,0 +1,67 @@
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
|
||||
import 'package:buzz/shared/relay/relay_provider.dart';
|
||||
|
||||
void main() {
|
||||
group('RelayConfig.baseUrl normalization', () {
|
||||
test('folds a wss:// community URL to https://', () {
|
||||
// Invite joins persist the relay URL straight off the invite link, which
|
||||
// deep_link.dart always emits as ws:// or wss://.
|
||||
final config = RelayConfig(baseUrl: 'wss://relay.example.com');
|
||||
expect(config.baseUrl, 'https://relay.example.com');
|
||||
});
|
||||
|
||||
test('folds a ws:// community URL to http://', () {
|
||||
final config = RelayConfig(baseUrl: 'ws://relay.example.com:3000');
|
||||
expect(config.baseUrl, 'http://relay.example.com:3000');
|
||||
});
|
||||
|
||||
test('leaves an https:// community URL untouched', () {
|
||||
// Device pairing rejects anything but https://, so these already conform.
|
||||
final config = RelayConfig(baseUrl: 'https://relay.example.com');
|
||||
expect(config.baseUrl, 'https://relay.example.com');
|
||||
});
|
||||
|
||||
test('leaves an http:// community URL untouched', () {
|
||||
final config = RelayConfig(baseUrl: 'http://localhost:3000');
|
||||
expect(config.baseUrl, 'http://localhost:3000');
|
||||
});
|
||||
|
||||
test('preserves a non-default port', () {
|
||||
final config = RelayConfig(baseUrl: 'wss://relay.example.com:8443');
|
||||
expect(config.baseUrl, 'https://relay.example.com:8443');
|
||||
});
|
||||
});
|
||||
|
||||
group('RelayConfig.wsUrl', () {
|
||||
test('keeps TLS for a relay joined by invite', () {
|
||||
// Regression: a wss:// base used to fall through to the non-https branch
|
||||
// and downgrade to ws://, dialing port 80 — which never connects on a
|
||||
// relay that only serves 443, and drops TLS everywhere else.
|
||||
final config = RelayConfig(baseUrl: 'wss://relay.example.com');
|
||||
expect(config.wsUrl, 'wss://relay.example.com');
|
||||
});
|
||||
|
||||
test('keeps TLS for a relay added by pairing', () {
|
||||
final config = RelayConfig(baseUrl: 'https://relay.example.com');
|
||||
expect(config.wsUrl, 'wss://relay.example.com');
|
||||
});
|
||||
|
||||
test('both onboarding paths agree on the same relay', () {
|
||||
final invited = RelayConfig(baseUrl: 'wss://relay.example.com');
|
||||
final paired = RelayConfig(baseUrl: 'https://relay.example.com');
|
||||
expect(invited.wsUrl, paired.wsUrl);
|
||||
expect(invited.baseUrl, paired.baseUrl);
|
||||
});
|
||||
|
||||
test('stays plaintext for local development', () {
|
||||
final config = RelayConfig(baseUrl: 'http://localhost:3000');
|
||||
expect(config.wsUrl, 'ws://localhost:3000');
|
||||
});
|
||||
|
||||
test('preserves a non-default port', () {
|
||||
final config = RelayConfig(baseUrl: 'wss://relay.example.com:8443');
|
||||
expect(config.wsUrl, 'wss://relay.example.com:8443');
|
||||
});
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user