diff --git a/.gitignore b/.gitignore index 2fda2ff..72458b8 100644 --- a/.gitignore +++ b/.gitignore @@ -61,3 +61,6 @@ windows/ # Playwright MCP browser session artifacts .playwright-mcp/ + +# Subagent persistent memory (local only) +.claude/agent-memory/ diff --git a/lib/providers/auth_provider.dart b/lib/providers/auth_provider.dart index bfb6985..224baf6 100644 --- a/lib/providers/auth_provider.dart +++ b/lib/providers/auth_provider.dart @@ -130,6 +130,10 @@ class AuthProvider with ChangeNotifier { print(' Handle: ${session.handle ?? trimmedHandle}'); print(' DID: ${session.did}'); } + } on SignInCancelledException { + // Cancel is not an error: don't record _error or flip auth state. + // The finally block still clears _isLoading. + rethrow; } catch (e) { _error = e.toString(); _isAuthenticated = false; diff --git a/lib/screens/auth/login_screen.dart b/lib/screens/auth/login_screen.dart index 919a166..61bb3f5 100644 --- a/lib/screens/auth/login_screen.dart +++ b/lib/screens/auth/login_screen.dart @@ -170,6 +170,30 @@ class _LoginScreenState extends State { ), ); } + // ignore: avoid_catching_errors + } on ArgumentError catch (e) { + // Handle validation failed (e.g. validateAndNormalizeHandle rejecting + // 'alice-.bsky.social', which passes the form validator). ArgumentError + // is an Error, not an Exception, so without this branch it would escape + // the catch chain below and crash. It's a user-input problem, not an + // app fault — show the validation message, skip Sentry. + if (mounted) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text( + e.message is String + ? e.message as String + : 'Invalid handle. Please check it and try again.', + style: GoogleFonts.nunito(fontWeight: FontWeight.w500), + ), + backgroundColor: AppColors.error, + behavior: SnackBarBehavior.floating, + shape: RoundedRectangleBorder( + borderRadius: BorderRadius.circular(12), + ), + ), + ); + } } on Exception catch (e, stackTrace) { // Log all sign-in errors to Sentry with categorization final errorString = e.toString().toLowerCase(); diff --git a/lib/services/coves_api_service.dart b/lib/services/coves_api_service.dart index 14bf73e..8a947b7 100644 --- a/lib/services/coves_api_service.dart +++ b/lib/services/coves_api_service.dart @@ -194,7 +194,10 @@ class CovesApiService { debugPrint('❌ API Error: ${error.message}'); if (error.response != null) { debugPrint(' Status: ${error.response?.statusCode}'); - debugPrint(' Data: ${error.response?.data}'); + // Response data can echo credentials — redact before printing + debugPrint( + redactBearerTokens(' Data: ${error.response?.data}'), + ); } } return handler.next(error); @@ -210,20 +213,27 @@ class CovesApiService { LogInterceptor( requestBody: true, responseBody: true, - logPrint: (obj) => debugPrint(_redactBearerTokens(obj.toString())), + logPrint: (obj) => debugPrint(redactBearerTokens(obj.toString())), ), ); } } + /// Matches a bearer scheme (case-insensitive) followed by any run of + /// non-whitespace characters. Greedy on purpose: a charset-based match + /// would leak the tail of tokens containing characters outside the set. + static final RegExp _bearerTokenPattern = RegExp( + r'Bearer\s+\S+', + caseSensitive: false, + ); + /// Replaces bearer token values with a placeholder so credentials never /// appear in logs. - static String _redactBearerTokens(String line) { - return line.replaceAll( - RegExp('Bearer [A-Za-z0-9._~+/=-]+'), - 'Bearer [REDACTED]', - ); + @visibleForTesting + static String redactBearerTokens(String line) { + return line.replaceAll(_bearerTokenPattern, 'Bearer [REDACTED]'); } + /// Maximum number of URIs per [getPosts] call, per the /// social.coves.community.post.get lexicon (`uris` has `maxLength: 25`). static const int maxPostGetUris = 25; @@ -438,7 +448,7 @@ class CovesApiService { 'limit': limit, }; - if (parentRkey != null) { + if (parentRkey != null && parentRkey.isNotEmpty) { queryParams['parentRkey'] = parentRkey; } @@ -1282,7 +1292,8 @@ class CovesApiService { debugPrint('❌ Failed to fetch $operation: ${e.message}'); if (e.response != null) { debugPrint(' Status: ${e.response?.statusCode}'); - debugPrint(' Data: ${e.response?.data}'); + // Response data can echo credentials — redact before printing + debugPrint(redactBearerTokens(' Data: ${e.response?.data}')); } } diff --git a/lib/services/coves_auth_service.dart b/lib/services/coves_auth_service.dart index ff61bcf..be2a87e 100644 --- a/lib/services/coves_auth_service.dart +++ b/lib/services/coves_auth_service.dart @@ -2,6 +2,7 @@ import 'dart:async'; import 'package:dio/dio.dart'; import 'package:flutter/foundation.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_secure_storage/flutter_secure_storage.dart'; import 'package:flutter_web_auth_2/flutter_web_auth_2.dart'; @@ -151,17 +152,31 @@ class CovesAuthService { // Open browser for OAuth flow // Backend redirects to custom scheme: social.coves:/callback - final resultUrl = await FlutterWebAuth2.authenticate( - url: loginUrl, - callbackUrlScheme: OAuthConfig.callbackScheme, - options: const FlutterWebAuth2Options( - preferEphemeral: true, // Don't persist browser session - timeout: 300, // 5 minutes - ), - ); + final String resultUrl; + try { + resultUrl = await FlutterWebAuth2.authenticate( + url: loginUrl, + callbackUrlScheme: OAuthConfig.callbackScheme, + options: const FlutterWebAuth2Options( + preferEphemeral: true, // Don't persist browser session + timeout: 300, // 5 minutes + ), + ); + } on PlatformException catch (e) { + // flutter_web_auth_2 signals user cancellation (browser dismissed, + // system auth sheet cancelled) as PlatformException(code: 'CANCELED') + // on every platform. Detect it via that typed signal, scoped to the + // authenticate call ONLY, so exceptions from parseCallbackUrl — + // which can embed server-controlled text like "cancelled" — are + // never reclassified as a quiet user cancel. + if (e.code == 'CANCELED') { + throw const SignInCancelledException(); + } + rethrow; + } if (kDebugMode) { - final redactedUrl = _redactSensitiveParams(resultUrl); + final redactedUrl = redactSensitiveParams(resultUrl); print('Received callback URL: $redactedUrl'); } @@ -194,12 +209,6 @@ class CovesAuthService { print('Sign-in failed: $e'); } - // Check for user cancellation (browser tab closed / system CANCELED) - if (e.toString().contains('CANCELED') || - e.toString().contains('cancelled')) { - throw const SignInCancelledException(); - } - throw Exception('Sign in failed: $e'); } } @@ -214,25 +223,73 @@ class CovesAuthService { /// [FormatException]. @visibleForTesting static CovesSession parseCallbackUrl(String resultUrl) { - final callbackUri = Uri.parse(resultUrl); + final Uri callbackUri; + final Map queryParameters; + try { + callbackUri = Uri.parse(resultUrl); + // Accessing queryParameters forces percent/UTF-8 decoding, which can + // throw ArgumentError (an Error, not Exception — it would escape every + // `on Exception` chain and crash) or a FormatException whose message + // echoes the raw URL (including the token). Convert both to a + // FormatException with a safe, fixed message. + queryParameters = callbackUri.queryParameters; + } on FormatException { + throw const FormatException('Malformed callback URL'); + // Intentional Error->Exception conversion at the untrusted-input + // boundary so malformed callbacks can't crash the app: + // ignore: avoid_catching_errors + } on ArgumentError { + throw const FormatException( + 'Malformed callback URL: invalid percent-encoding', + ); + } - final oauthError = callbackUri.queryParameters['error']; - if (oauthError != null && oauthError.isNotEmpty) { + // The error/error_description params are attacker-influenceable (any app + // can fire the social.coves custom scheme), so sanitize before embedding + // them in exception messages that flow to Sentry/logs. + final oauthError = _sanitizeCallbackText(queryParameters['error']); + if (oauthError != null) { if (kDebugMode) { - // Error codes/descriptions contain no secrets — safe to log. + // Sanitized error codes contain no secrets — safe to log. print('OAuth callback returned error: $oauthError'); } if (oauthError == 'access_denied') { throw const SignInCancelledException(); } - final description = callbackUri.queryParameters['error_description']; - final detail = (description == null || description.isEmpty) - ? '' - : ' ($description)'; + final description = _sanitizeCallbackText( + queryParameters['error_description'], + ); + final detail = description == null ? '' : ' ($description)'; throw Exception('Authorization server error: $oauthError$detail'); } - return CovesSession.fromCallbackUri(callbackUri); + try { + return CovesSession.fromCallbackUri(callbackUri); + // ignore: avoid_catching_errors + } on ArgumentError { + // Uri.decodeComponent inside fromCallbackUri throws ArgumentError on + // malformed percent-encoding (e.g. token=%ZZ) — convert it to a + // catchable FormatException so it can't crash the app. + throw const FormatException( + 'Malformed callback URL: invalid percent-encoding', + ); + } + } + + /// Sanitize attacker-influenceable callback text before it is embedded in + /// exception messages: strip control characters (including newlines) and + /// cap the length so hostile callbacks can't inject into Sentry/logs. + /// + /// Returns null for null, empty, or control-character-only input. + static String? _sanitizeCallbackText(String? value) { + if (value == null) { + return null; + } + var sanitized = value.replaceAll(RegExp(r'[\x00-\x1f\x7f]'), ''); + if (sanitized.length > 200) { + sanitized = '${sanitized.substring(0, 200)}...'; + } + return sanitized.isEmpty ? null : sanitized; } /// Restore a previous session from secure storage @@ -586,7 +643,8 @@ class CovesAuthService { /// /// Non-sensitive params like DID, handle, and session_id are preserved /// as they're useful for debugging without being security-sensitive. - String _redactSensitiveParams(String url) { + @visibleForTesting + static String redactSensitiveParams(String url) { // Replace token=xxx with token=[REDACTED] // Matches token= followed by any non-whitespace, non-ampersand characters return url.replaceAllMapped( diff --git a/test/providers/auth_provider_test.dart b/test/providers/auth_provider_test.dart index 5b80fc2..4e05744 100644 --- a/test/providers/auth_provider_test.dart +++ b/test/providers/auth_provider_test.dart @@ -95,6 +95,32 @@ void main() { expect(authProvider.isAuthenticated, false); }); + test( + 'should rethrow SignInCancelledException without setting error state', + () async { + when( + mockAuthService.signIn('alice.bsky.social'), + ).thenThrow(const SignInCancelledException()); + + // Record the error value at every notification: no notification + // should ever carry an error state for a user cancel. + final observedErrors = []; + authProvider.addListener(() { + observedErrors.add(authProvider.error); + }); + + await expectLater( + authProvider.signIn('alice.bsky.social'), + throwsA(isA()), + ); + + expect(authProvider.error, null); + expect(authProvider.isLoading, false); + expect(authProvider.isAuthenticated, false); + expect(observedErrors, everyElement(isNull)); + }, + ); + test('should handle sign in errors', () async { when( mockAuthService.signIn('invalid.handle'), diff --git a/test/services/coves_api_service_community_test.dart b/test/services/coves_api_service_community_test.dart index 81b486e..40231cd 100644 --- a/test/services/coves_api_service_community_test.dart +++ b/test/services/coves_api_service_community_test.dart @@ -240,10 +240,16 @@ void main() { }, ); - expect( - () => apiService.listCommunities(), - throwsA(isA()), - ); + // Assert retry exhaustion actually happened so fixture drift can't + // silently disable the RetryInterceptor again. + try { + await apiService.listCommunities(); + fail('Expected NetworkException'); + } on NetworkException catch (e) { + final dioError = e.originalError as DioException; + expect(dioError.message, contains('failed after 2 retries')); + expect(dioError.requestOptions.extra['retriesExhausted'], isTrue); + } }); }); @@ -476,13 +482,19 @@ void main() { }, ); - expect( - () => apiService.createPost( + // Assert retry exhaustion actually happened so fixture drift can't + // silently disable the RetryInterceptor again. + try { + await apiService.createPost( community: 'did:plc:community1', title: 'Test', - ), - throwsA(isA()), - ); + ); + fail('Expected NetworkException'); + } on NetworkException catch (e) { + final dioError = e.originalError as DioException; + expect(dioError.message, contains('failed after 2 retries')); + expect(dioError.requestOptions.extra['retriesExhausted'], isTrue); + } }); }); } diff --git a/test/services/coves_api_service_redaction_test.dart b/test/services/coves_api_service_redaction_test.dart new file mode 100644 index 0000000..7c4a10d --- /dev/null +++ b/test/services/coves_api_service_redaction_test.dart @@ -0,0 +1,115 @@ +import 'package:coves_flutter/services/api_exceptions.dart'; +import 'package:coves_flutter/services/coves_api_service.dart'; +import 'package:dio/dio.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http_mock_adapter/http_mock_adapter.dart'; + +/// Tests for bearer token redaction in CovesApiService debug logs. +/// +/// kDebugMode is true under `flutter test`, so the LogInterceptor added in +/// the CovesApiService constructor is active. debugPrint is swapped for a +/// capturing closure so every log line a real mocked request produces can +/// be asserted on (restored in tearDown). +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + // Contains '!' — a character outside the old regex charset — to pin the + // greedy non-whitespace match (the old pattern leaked the token tail). + const token = 'abc!def.secret~token'; + + group('CovesApiService - bearer token redaction', () { + late Dio dio; + late DioAdapter dioAdapter; + late CovesApiService apiService; + late List logLines; + late DebugPrintCallback originalDebugPrint; + + setUp(() { + logLines = []; + originalDebugPrint = debugPrint; + debugPrint = (String? message, {int? wrapWidth}) { + logLines.add(message ?? ''); + }; + + dio = Dio(BaseOptions(baseUrl: 'https://api.test.coves.social')); + dioAdapter = DioAdapter(dio: dio); + apiService = CovesApiService( + dio: dio, + tokenGetter: () async => token, + ); + }); + + tearDown(() { + debugPrint = originalDebugPrint; + apiService.dispose(); + }); + + test('redacts the Authorization header in request logs', () async { + dioAdapter.onGet( + '/xrpc/social.coves.community.list', + (server) => server.reply(200, {'communities': [], 'cursor': null}), + queryParameters: {'limit': 50, 'sort': 'popular'}, + ); + + await apiService.listCommunities(); + + final output = logLines.join('\n'); + expect(output, contains('Bearer [REDACTED]')); + expect(output, isNot(contains(token))); + }); + + test('redacts bearer tokens echoed in error response data', () async { + dioAdapter.onGet( + '/xrpc/social.coves.community.list', + (server) => server.reply(500, { + 'error': 'InternalServerError', + 'message': 'debug echo: Bearer $token from upstream', + }), + queryParameters: {'limit': 50, 'sort': 'popular'}, + ); + + await expectLater( + apiService.listCommunities(), + throwsA(isA()), + ); + + final output = logLines.join('\n'); + // The onError ' Data:' debugPrint and the LogInterceptor both log + // the response body — neither may leak the raw token. + expect(output, contains('Data:')); + expect(output, contains('Bearer [REDACTED]')); + expect(output, isNot(contains(token))); + }); + }); + + group('CovesApiService.redactBearerTokens', () { + test('greedily redacts tokens with chars outside the old charset', () { + expect( + CovesApiService.redactBearerTokens('Authorization: Bearer $token'), + 'Authorization: Bearer [REDACTED]', + ); + }); + + test('is case-insensitive', () { + expect( + CovesApiService.redactBearerTokens('authorization: bearer $token'), + 'authorization: Bearer [REDACTED]', + ); + }); + + test('handles tab whitespace between scheme and token', () { + expect( + CovesApiService.redactBearerTokens('Bearer\t$token trailing'), + 'Bearer [REDACTED] trailing', + ); + }); + + test('redacts every occurrence in a line', () { + expect( + CovesApiService.redactBearerTokens('Bearer aaa!x and BEARER bbb!y'), + 'Bearer [REDACTED] and Bearer [REDACTED]', + ); + }); + }); +} diff --git a/test/services/coves_api_service_test.dart b/test/services/coves_api_service_test.dart index f223798..eb5bc24 100644 --- a/test/services/coves_api_service_test.dart +++ b/test/services/coves_api_service_test.dart @@ -285,6 +285,95 @@ void main() { expect(response, isA()); }); + test('should send parentRkey as a query parameter when provided', () async { + const postUri = 'at://did:plc:test/social.coves.post.record/123'; + const parentRkey = '3kparentrkey'; + + final mockResponse = { + 'post': {'uri': postUri}, + 'cursor': null, + 'comments': [], + }; + + // DioAdapter matches the full query-parameter map, so this only + // replies if 'parentRkey' actually reaches the wire with this value. + dioAdapter.onGet( + '/xrpc/social.coves.community.comment.getComments', + (server) => server.reply(200, mockResponse), + queryParameters: { + 'post': postUri, + 'sort': 'hot', + 'depth': 10, + 'limit': 50, + 'parentRkey': parentRkey, + }, + ); + + final response = await apiService.getComments( + postUri: postUri, + parentRkey: parentRkey, + ); + + expect(response, isA()); + }); + + test('should omit parentRkey from query when null', () async { + const postUri = 'at://did:plc:test/social.coves.post.record/123'; + + final mockResponse = { + 'post': {'uri': postUri}, + 'cursor': null, + 'comments': [], + }; + + // Mock has no 'parentRkey' key — the request only matches if the + // parameter is omitted entirely. + dioAdapter.onGet( + '/xrpc/social.coves.community.comment.getComments', + (server) => server.reply(200, mockResponse), + queryParameters: { + 'post': postUri, + 'sort': 'hot', + 'depth': 10, + 'limit': 50, + }, + ); + + final response = await apiService.getComments(postUri: postUri); + + expect(response, isA()); + }); + + test('should omit parentRkey from query when empty', () async { + const postUri = 'at://did:plc:test/social.coves.post.record/123'; + + final mockResponse = { + 'post': {'uri': postUri}, + 'cursor': null, + 'comments': [], + }; + + // Mock has no 'parentRkey' key — the request only matches if the + // empty string is dropped instead of sent as parentRkey=. + dioAdapter.onGet( + '/xrpc/social.coves.community.comment.getComments', + (server) => server.reply(200, mockResponse), + queryParameters: { + 'post': postUri, + 'sort': 'hot', + 'depth': 10, + 'limit': 50, + }, + ); + + final response = await apiService.getComments( + postUri: postUri, + parentRkey: '', + ); + + expect(response, isA()); + }); + test('should handle 404 error', () async { const postUri = 'at://did:plc:test/social.coves.post.record/nonexistent'; @@ -363,10 +452,16 @@ void main() { }, ); - expect( - () => apiService.getComments(postUri: postUri), - throwsA(isA()), - ); + // Assert retry exhaustion actually happened so fixture drift can't + // silently disable the RetryInterceptor again. + try { + await apiService.getComments(postUri: postUri); + fail('Expected NetworkException'); + } on NetworkException catch (e) { + final dioError = e.originalError as DioException; + expect(dioError.message, contains('failed after 2 retries')); + expect(dioError.requestOptions.extra['retriesExhausted'], isTrue); + } }); test('should handle network connection error', () async { @@ -401,10 +496,16 @@ void main() { }, ); - expect( - () => apiService.getComments(postUri: postUri), - throwsA(isA()), - ); + // Assert retry exhaustion actually happened so fixture drift can't + // silently disable the RetryInterceptor again. + try { + await apiService.getComments(postUri: postUri); + fail('Expected NetworkException'); + } on NetworkException catch (e) { + final dioError = e.originalError as DioException; + expect(dioError.message, contains('failed after 2 retries')); + expect(dioError.requestOptions.extra['retriesExhausted'], isTrue); + } }); test('should handle invalid JSON response', () async { diff --git a/test/services/coves_auth_service_callback_test.dart b/test/services/coves_auth_service_callback_test.dart index a61780c..832179d 100644 --- a/test/services/coves_auth_service_callback_test.dart +++ b/test/services/coves_auth_service_callback_test.dart @@ -52,5 +52,116 @@ void main() { throwsA(isA()), ); }); + + test('error param wins over valid token params (error precedence)', () { + // A hostile app firing the social.coves scheme could pack both valid + // session params AND an error code into one callback. The error must + // win — never silently mint a session from a callback carrying one. + expect( + () => CovesAuthService.parseCallbackUrl( + 'social.coves:/callback?token=abc123&did=did:plc:test123' + '&session_id=sess456&handle=test.user&error=access_denied', + ), + throwsA(isA()), + ); + }); + + test( + 'malformed percent-encoding (token=%ZZ) throws FormatException, ' + 'not ArgumentError', + () { + // Uri.decodeComponent('%ZZ') throws ArgumentError — an Error, not an + // Exception — which would escape every catch chain and crash the app. + // parseCallbackUrl must convert it to a catchable FormatException. + expect( + () => CovesAuthService.parseCallbackUrl( + 'social.coves:/callback?token=%ZZ&did=did:plc:test123' + '&session_id=sess456', + ), + throwsA(isA()), + ); + }, + ); + + test( + 'truncated UTF-8 percent-encoding throws FormatException with a safe ' + 'message that does not echo the raw URL', + () { + expect( + () => CovesAuthService.parseCallbackUrl( + 'social.coves:/callback?token=%E0%A4%A&did=did:plc:test123' + '&session_id=sess456', + ), + throwsA( + predicate( + (e) => + e is FormatException && !e.toString().contains('did:plc'), + ), + ), + ); + }, + ); + + test('totally garbled URL throws FormatException, not an Error', () { + expect( + // Unterminated IPv6 host — Uri.parse itself rejects this. + () => CovesAuthService.parseCallbackUrl('http://[::garbled'), + throwsA(isA()), + ); + }); + + test( + 'non-access_denied error mentioning "cancelled" surfaces as a real ' + 'error with the formatted description, not a quiet cancel', + () { + // Reviewer scenario: server-controlled text containing "cancelled" + // must never be reclassified as a user cancel. + expect( + () => CovesAuthService.parseCallbackUrl( + 'social.coves:/callback?error=temporarily_unavailable' + '&error_description=request+was+cancelled+by+upstream', + ), + throwsA( + predicate( + (e) => + e is Exception && + e is! SignInCancelledException && + e.toString().contains( + 'Authorization server error: temporarily_unavailable ' + '(request was cancelled by upstream)', + ), + ), + ), + ); + }, + ); + + test('sanitizes error_description: strips control chars, caps length', () { + final longDescription = 'a' * 500; + final url = + 'social.coves:/callback?error=server_error' + '&error_description=' + '${Uri.encodeComponent('evil\n\r\x01payload $longDescription')}'; + + expect( + () => CovesAuthService.parseCallbackUrl(url), + throwsA( + predicate((e) { + final message = e.toString(); + return e is Exception && + e is! SignInCancelledException && + // Control characters (incl. newlines) are stripped, so + // hostile callbacks can't inject lines into Sentry/logs. + !message.contains('\n') && + !message.contains('\r') && + !message.contains('\x01') && + message.contains('evilpayload') && + // Description capped at ~200 chars plus ellipsis. + message.length < 300 && + message.contains('...'); + }), + ), + ); + }); }); } diff --git a/test/services/coves_auth_service_redaction_test.dart b/test/services/coves_auth_service_redaction_test.dart index 4f8a790..2a9c1fc 100644 --- a/test/services/coves_auth_service_redaction_test.dart +++ b/test/services/coves_auth_service_redaction_test.dart @@ -6,28 +6,22 @@ import 'package:flutter_test/flutter_test.dart'; /// /// Verifies that sensitive parameters (tokens) are properly redacted /// from debug logs while preserving useful debugging information. +/// +/// Exercises the real production helper +/// [CovesAuthService.redactSensitiveParams] so regressions in the +/// production regex are caught here. void main() { - setUp(() { - // Reset singleton state before each test - CovesAuthService.resetInstance(); - }); + // Reset singleton state around each test + setUp(CovesAuthService.resetInstance); - tearDown(() { - CovesAuthService.resetInstance(); - }); + tearDown(CovesAuthService.resetInstance); - group('_redactSensitiveParams', () { + group('redactSensitiveParams', () { test('should redact token parameter from callback URL', () { const testUrl = 'social.coves:/callback?token=sealed_token_abc123&did=did:plc:test123&session_id=sess-456&handle=alice.bsky.social'; - // Use reflection to call private method - // Since we can't directly call private methods, we'll test the behavior - // through the public signIn method which logs the redacted URL - final redacted = testUrl.replaceAllMapped( - RegExp(r'token=([^&\s]+)'), - (match) => 'token=[REDACTED]', - ); + final redacted = CovesAuthService.redactSensitiveParams(testUrl); expect( redacted, @@ -41,10 +35,7 @@ void main() { const testUrl = 'social.coves:/callback?token=sealed_token_abc123&did=did:plc:test123&session_id=sess-456&handle=alice.bsky.social'; - final redacted = testUrl.replaceAllMapped( - RegExp(r'token=([^&\s]+)'), - (match) => 'token=[REDACTED]', - ); + final redacted = CovesAuthService.redactSensitiveParams(testUrl); expect(redacted, contains('did=did:plc:test123')); expect(redacted, contains('session_id=sess-456')); @@ -57,10 +48,7 @@ void main() { const testUrl = 'social.coves:/callback?token=first_token&did=did:plc:test'; - final redacted = testUrl.replaceAllMapped( - RegExp(r'token=([^&\s]+)'), - (match) => 'token=[REDACTED]', - ); + final redacted = CovesAuthService.redactSensitiveParams(testUrl); expect( redacted, @@ -72,10 +60,7 @@ void main() { const testUrl = 'social.coves:/callback?did=did:plc:test&token=last_token'; - final redacted = testUrl.replaceAllMapped( - RegExp(r'token=([^&\s]+)'), - (match) => 'token=[REDACTED]', - ); + final redacted = CovesAuthService.redactSensitiveParams(testUrl); expect( redacted, @@ -86,10 +71,7 @@ void main() { test('should handle token as only parameter', () { const testUrl = 'social.coves:/callback?token=only_token'; - final redacted = testUrl.replaceAllMapped( - RegExp(r'token=([^&\s]+)'), - (match) => 'token=[REDACTED]', - ); + final redacted = CovesAuthService.redactSensitiveParams(testUrl); expect(redacted, 'social.coves:/callback?token=[REDACTED]'); }); @@ -98,10 +80,7 @@ void main() { const testUrl = 'social.coves:/callback?token=encoded%2Btoken%3D123&did=did:plc:test'; - final redacted = testUrl.replaceAllMapped( - RegExp(r'token=([^&\s]+)'), - (match) => 'token=[REDACTED]', - ); + final redacted = CovesAuthService.redactSensitiveParams(testUrl); expect( redacted, @@ -113,13 +92,10 @@ void main() { test('should handle long token values', () { const longToken = 'very_long_sealed_token_with_many_characters_1234567890abcdef'; - final testUrl = + const testUrl = 'social.coves:/callback?token=$longToken&did=did:plc:test'; - final redacted = testUrl.replaceAllMapped( - RegExp(r'token=([^&\s]+)'), - (match) => 'token=[REDACTED]', - ); + final redacted = CovesAuthService.redactSensitiveParams(testUrl); expect( redacted, @@ -132,10 +108,7 @@ void main() { const testUrl = 'social.coves:/callback?did=did:plc:test&handle=alice.bsky.social'; - final redacted = testUrl.replaceAllMapped( - RegExp(r'token=([^&\s]+)'), - (match) => 'token=[REDACTED]', - ); + final redacted = CovesAuthService.redactSensitiveParams(testUrl); // Should remain unchanged if no token present expect(redacted, testUrl); @@ -144,10 +117,7 @@ void main() { test('should handle malformed URLs gracefully', () { const testUrl = 'social.coves:/callback?token='; - final redacted = testUrl.replaceAllMapped( - RegExp(r'token=([^&\s]+)'), - (match) => 'token=[REDACTED]', - ); + final redacted = CovesAuthService.redactSensitiveParams(testUrl); // Empty token value - regex won't match, URL stays the same expect(redacted, testUrl); diff --git a/test/services/coves_auth_service_test.dart b/test/services/coves_auth_service_test.dart index f6bb8d6..e9f5346 100644 --- a/test/services/coves_auth_service_test.dart +++ b/test/services/coves_auth_service_test.dart @@ -51,8 +51,9 @@ void main() { test('should throw appropriate error when user cancels sign-in', () async { // Note: FlutterWebAuth2.authenticate is not easily mockable as it's a static method // This test documents expected behavior when authentication is cancelled - // In practice, this would throw with CANCELED/cancelled in the message - // The actual implementation catches this and rethrows with user-friendly message + // In practice, flutter_web_auth_2 throws PlatformException(code: 'CANCELED') + // on every platform; signIn catches that typed signal (scoped to the + // authenticate call only) and throws SignInCancelledException // This test would require integration testing or a wrapper around FlutterWebAuth2 // Skipping for now as it requires more complex mocking infrastructure