diff --git a/crdt_sync_dart/lib/src/firebase_auth_rest.dart b/crdt_sync_dart/lib/src/firebase_auth_rest.dart index 011cc74..166569e 100644 --- a/crdt_sync_dart/lib/src/firebase_auth_rest.dart +++ b/crdt_sync_dart/lib/src/firebase_auth_rest.dart @@ -221,7 +221,7 @@ class FirebaseTokenProvider { body: jsonEncode(payload), ); } on http.ClientException catch (exc) { - throw FirebaseAuthError('network error trying to $what: $exc'); + throw FirebaseAuthError('network error trying to $what: ${exc.message}'); } if (res.statusCode < 200 || res.statusCode >= 300) { throw FirebaseAuthError( diff --git a/crdt_sync_dart/lib/src/firebase_client.dart b/crdt_sync_dart/lib/src/firebase_client.dart index 819645a..a9be3a6 100644 --- a/crdt_sync_dart/lib/src/firebase_client.dart +++ b/crdt_sync_dart/lib/src/firebase_client.dart @@ -79,7 +79,9 @@ class FirebaseRestClient implements RemoteStore, BulkMapReader { try { return await _http.get(await _uri(path, query: query)); } on http.ClientException catch (exc) { - throw FirebaseSyncError('network error reading $path: $exc'); + // .message, never the exception: its toString prints the request URL, + // whose query is `?auth=`. Same at every catch below. + throw FirebaseSyncError('network error reading $path: ${exc.message}'); } } @@ -126,7 +128,7 @@ class FirebaseRestClient implements RemoteStore, BulkMapReader { try { res = await _http.put(await _uri(path), body: jsonEncode(text)); } on http.ClientException catch (exc) { - throw FirebaseSyncError('network error writing $path: $exc'); + throw FirebaseSyncError('network error writing $path: ${exc.message}'); } if (res.statusCode < 200 || res.statusCode >= 300) { _raise('writing $path', res); @@ -143,7 +145,7 @@ class FirebaseRestClient implements RemoteStore, BulkMapReader { try { res = await _http.patch(await _uri(path), body: jsonEncode(values)); } on http.ClientException catch (exc) { - throw FirebaseSyncError('network error patching $path: $exc'); + throw FirebaseSyncError('network error patching $path: ${exc.message}'); } if (res.statusCode < 200 || res.statusCode >= 300) { _raise('patching $path', res); @@ -178,7 +180,7 @@ class FirebaseRestClient implements RemoteStore, BulkMapReader { try { res = await _http.delete(await _uri(path)); } on http.ClientException catch (exc) { - throw FirebaseSyncError('network error deleting $path: $exc'); + throw FirebaseSyncError('network error deleting $path: ${exc.message}'); } if (res.statusCode < 200 || res.statusCode >= 300) { _raise('deleting $path', res); diff --git a/crdt_sync_dart/test/firebase_network_error_redaction_test.dart b/crdt_sync_dart/test/firebase_network_error_redaction_test.dart new file mode 100644 index 0000000..d3996a6 --- /dev/null +++ b/crdt_sync_dart/test/firebase_network_error_redaction_test.dart @@ -0,0 +1,65 @@ +// A real ClientException carries the request URL, and its toString prints +// it: `uri=https:///.json?auth=` for RTDB, `?key=` for auth. Error messages end up in UIs and logs, so neither may +// ever contain the URL's query. +import 'package:crdt_sync/crdt_sync.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart' as http_testing; +import 'package:test/test.dart'; + +/// Fails every request the way IOClient does: message plus request URL. +http.Client _offline() => http_testing.MockClient( + (request) async => + throw http.ClientException('Failed host lookup', request.url), +); + +FirebaseRestClient _client() => FirebaseRestClient( + databaseUrl: 'https://db.example', + auth: FirebaseTokenProvider( + apiKey: 'fake-api-key', + store: InMemoryCredentialStore( + FirebaseCredentials( + idToken: 'SECRET-ID-TOKEN', + refreshToken: 'refresh-token', + expiresAt: DateTime.utc(2099), + ), + ), + httpClient: _offline(), + ), + httpClient: _offline(), +); + +Matcher _redacted(String secret) => throwsA( + isA() + .having((e) => '$e', 'text', contains('Failed host lookup')) + .having((e) => '$e', 'text', isNot(contains(secret))), +); + +void main() { + const token = 'SECRET-ID-TOKEN'; + final ops = Function(FirebaseRestClient)>{ + 'getFileText': (c) => c.getFileText('ns/a.json'), + 'listDirectory': (c) => c.listDirectory('ns'), + 'getStringMap': (c) => c.getStringMap('ns/revs'), + 'putFileText': (c) => c.putFileText('ns/a.json', 'x', message: 'm'), + 'patchValues': (c) => c.patchValues('ns/revs', {'a': '1'}), + 'deleteFile': (c) => c.deleteFile('ns/a.json'), + }; + for (final MapEntry(key: name, value: op) in ops.entries) { + test('$name: a network error never shows the ID token', () { + expect(op(_client()), _redacted(token)); + }); + } + + test('auth: a network error never shows the API key', () { + final provider = FirebaseTokenProvider( + apiKey: 'SECRET-API-KEY', + store: InMemoryCredentialStore(), + httpClient: _offline(), + ); + expect( + provider.signIn(email: 'a@b.c', password: 'x'), + _redacted('SECRET-API-KEY'), + ); + }); +}