diff --git a/lib/providers/user_profile_provider.dart b/lib/providers/user_profile_provider.dart index ec8b892..bbd3e3b 100644 --- a/lib/providers/user_profile_provider.dart +++ b/lib/providers/user_profile_provider.dart @@ -190,13 +190,6 @@ class UserProfileProvider with ChangeNotifier { if (kDebugMode) { debugPrint('❌ Failed to load profile: ${e.message}'); } - } on FormatException catch (e) { - _isLoadingProfile = false; - _profileError = 'Invalid data received from server'; - - if (kDebugMode) { - debugPrint('❌ Format error loading profile: $e'); - } } on Exception catch (e) { // Catch-all for other exceptions _isLoadingProfile = false; @@ -319,16 +312,6 @@ class UserProfileProvider with ChangeNotifier { if (kDebugMode) { debugPrint('❌ Failed to load author posts: ${e.message}'); } - } on FormatException catch (e) { - _postsState = currentState.copyWith( - error: 'Invalid data received from server', - isLoading: false, - isLoadingMore: false, - ); - - if (kDebugMode) { - debugPrint('❌ Format error loading posts: $e'); - } } on Exception catch (e) { // Catch-all for other exceptions _postsState = currentState.copyWith( @@ -459,16 +442,6 @@ class UserProfileProvider with ChangeNotifier { if (kDebugMode) { debugPrint('❌ Failed to load author comments: ${e.message}'); } - } on FormatException catch (e) { - _commentsState = currentState.copyWith( - error: 'Invalid data received from server', - isLoading: false, - isLoadingMore: false, - ); - - if (kDebugMode) { - debugPrint('❌ Format error loading comments: $e'); - } } on Exception catch (e) { _commentsState = currentState.copyWith( error: 'Failed to load comments. Please try again.', diff --git a/lib/services/api_exceptions.dart b/lib/services/api_exceptions.dart index d61f72b..aea97c2 100644 --- a/lib/services/api_exceptions.dart +++ b/lib/services/api_exceptions.dart @@ -4,10 +4,16 @@ /// This allows better error handling and user-friendly error messages. /// /// [ApiException.fromDioError] is the single canonical mapping from -/// [DioException] to these types — services must not hand-roll their own -/// status-code switches. +/// [DioException] to these types, and [mapDioException] is the entry point +/// services use (it adds the redacted debug logging). Services must not +/// hand-roll their own status-code switches; substituting friendlier +/// message *copy* for an expected status is fine (see +/// `CommentService.deleteComment`), but the exception types must stay +/// within this taxonomy. library; +import 'dart:io'; + import 'package:dio/dio.dart'; import 'package:flutter/foundation.dart'; @@ -27,17 +33,27 @@ class ApiException implements Exception { /// /// Without a response, the Dio error type picks the type: timeouts and /// connection failures → [NetworkException], DNS resolution failures → - /// [FederationException] (the PDS may be unreachable). + /// [FederationException] (the PDS may be unreachable), cancelled + /// requests → a plain [ApiException]. factory ApiException.fromDioError(DioException error) { final response = error.response; final statusCode = response?.statusCode; if (response != null && statusCode != null) { - // Handle both JSON error responses and plain text responses + // Handle both JSON error responses and plain text responses. + // Read the fields defensively — a hostile or buggy server can put + // non-string values in either, and a TypeError thrown here would + // escape the whole ApiException taxonomy. String? message; final data = response.data; if (data is Map) { - message = data['message'] as String? ?? data['error'] as String?; + final rawMessage = data['message']; + final rawError = data['error']; + if (rawMessage is String && rawMessage.isNotEmpty) { + message = rawMessage; + } else if (rawError is String && rawError.isNotEmpty) { + message = rawError; + } } else if (data is String && data.isNotEmpty) { message = data; } @@ -79,7 +95,7 @@ class ApiException implements Exception { ); case DioExceptionType.connectionError: // Could be federation issue if it's a PDS connection failure - if (error.message?.contains('Failed host lookup') ?? false) { + if (_isDnsFailure(error)) { return FederationException( 'Failed to connect to PDS. Server may be unreachable', originalError: error, @@ -96,28 +112,62 @@ class ApiException implements Exception { case DioExceptionType.badResponse: // Response missing or without a status code return ApiException( - 'Bad response from server: ${error.message}', + 'Bad response from server: ${error.message ?? 'no details'}', originalError: error, ); case DioExceptionType.unknown: - return NetworkException( - 'Network error: ${error.message ?? 'unknown'}', + // Dio uses `unknown` for more than transport failures (decode + // errors, interceptor throws, ...). Classify by the wrapped error + // so a truncated 200 body isn't blamed on the user's connection. + final inner = error.error; + if (inner is FormatException) { + return ApiException( + 'Failed to parse server response', + originalError: error, + ); + } + if (inner is IOException) { + return NetworkException( + 'Network error: ${error.message ?? inner}', + originalError: error, + ); + } + return ApiException( + 'Unknown error: ${error.message ?? inner ?? 'no details'}', originalError: error, ); } } + /// True when a connection error is a DNS resolution failure. Prefers the + /// typed [SocketException] over Dio's message text, which varies by + /// platform; the substring check remains as a fallback. + static bool _isDnsFailure(DioException error) { + final inner = error.error; + if (inner is SocketException && + inner.message.contains('Failed host lookup')) { + return true; + } + return error.message?.contains('Failed host lookup') ?? false; + } + final String message; final int? statusCode; + + /// The underlying error, typically the mapped [DioException]. + /// + /// May embed the full request — including a live Authorization header — + /// so never log or serialize this field. Log [message], or pass response + /// data through [redactBearerTokens] first. final dynamic originalError; @override String toString() => message; } -/// Maps [error] to a typed [ApiException] after logging it (with bearer -/// tokens redacted) in debug builds. [operation] labels the log line, -/// e.g. 'fetch timeline'. +/// Maps [error] to a typed [ApiException] after logging it in debug builds +/// (response data is passed through [redactBearerTokens] first). +/// [operation] labels the log line, e.g. 'fetch timeline'. ApiException mapDioException(DioException error, {required String operation}) { if (kDebugMode) { debugPrint('❌ Failed to $operation: ${error.message}'); diff --git a/lib/services/auth_interceptor.dart b/lib/services/auth_interceptor.dart index e04512f..52d4102 100644 --- a/lib/services/auth_interceptor.dart +++ b/lib/services/auth_interceptor.dart @@ -1,8 +1,6 @@ import 'package:dio/dio.dart'; import 'package:flutter/foundation.dart'; -import 'log_redaction.dart'; - /// Creates a Dio interceptor that handles authentication and automatic /// token refresh on 401 errors. /// @@ -148,24 +146,24 @@ InterceptorsWrapper createAuthInterceptor({ '❌ $serviceName: Token refresh failed, propagating error', ); } - } on Exception catch (e) { - // Same rule as above: an exception here (from the refresher or - // from retrying the original request) is not evidence the session - // is dead, so never sign out - just propagate the error. + } on Object catch (e) { + // Same rule as above: an error here (from the refresher or from + // retrying the original request) is not evidence the session is + // dead, so never sign out - just propagate. Object, not + // Exception: a TypeError/StateError from the auth provider must + // not escape this async handler, or it would bypass the error + // taxonomy and surface as a misclassified network error instead + // of the original 401. if (kDebugMode) { debugPrint('❌ $serviceName: Error during token refresh: $e'); } } } - // Log the error for debugging + // One brief line here; mapDioException prints the status and + // redacted body when the owning service maps this error. if (kDebugMode) { debugPrint('❌ $serviceName API Error: ${error.message}'); - if (error.response != null) { - debugPrint(' Status: ${error.response?.statusCode}'); - // Response data can echo credentials — redact before printing - debugPrint(redactBearerTokens(' Data: ${error.response?.data}')); - } } return handler.next(error); }, diff --git a/lib/services/comment_service.dart b/lib/services/comment_service.dart index 803b975..2115ae1 100644 --- a/lib/services/comment_service.dart +++ b/lib/services/comment_service.dart @@ -10,7 +10,7 @@ import 'retry_interceptor.dart'; /// Comment Service /// -/// Handles comment creation through the Coves backend. +/// Handles comment creation and deletion through the Coves backend. /// /// **Architecture with Backend OAuth**: /// With sealed tokens, the client cannot write directly to the user's PDS @@ -23,8 +23,9 @@ import 'retry_interceptor.dart'; /// 2. Uses stored DPoP keys to sign requests /// 3. Writes to the user's PDS on their behalf /// -/// **Backend Endpoint**: +/// **Backend Endpoints**: /// - POST /xrpc/social.coves.community.comment.create +/// - POST /xrpc/social.coves.community.comment.delete class CommentService { CommentService({ Future Function()? sessionGetter, @@ -169,7 +170,8 @@ class CommentService { /// Throws: /// - AuthenticationException if not authenticated /// - ApiException with 'You can only delete your own comments' if not - /// the comment author + /// the comment author (403) + /// - NotFoundException if the comment no longer exists (404) /// - ApiException for other errors Future deleteComment({required String uri}) async { try { @@ -194,8 +196,9 @@ class CommentService { debugPrint('✅ Comment deleted successfully'); } } on DioException catch (e) { - // Friendlier copy than the server's for the two expected outcomes; - // everything else goes through the canonical mapper. + // Map first so every failure gets the standard (redacted) debug log, + // then substitute friendlier copy for the two expected outcomes. + final mapped = mapDioException(e, operation: 'delete comment'); if (e.response?.statusCode == 403) { throw ApiException( 'You can only delete your own comments', @@ -209,7 +212,7 @@ class CommentService { originalError: e, ); } - throw mapDioException(e, operation: 'delete comment'); + throw mapped; } on AuthenticationException { rethrow; } on NotFoundException { diff --git a/lib/services/coves_api_service.dart b/lib/services/coves_api_service.dart index bf00e17..bf3e4b4 100644 --- a/lib/services/coves_api_service.dart +++ b/lib/services/coves_api_service.dart @@ -125,7 +125,10 @@ class CovesApiService { throw mapDioException(e, operation: operation); } on ApiException { rethrow; - } catch (e) { + } on Object catch (e) { + // Object on purpose: Errors from [parse] (TypeError from a bad cast, + // etc.) are deliberately degraded to a parse ApiException so callers + // only ever see the ApiException taxonomy. if (kDebugMode) { debugPrint('❌ Error parsing $operation response: $e'); } @@ -305,7 +308,10 @@ class CovesApiService { /// Parameters: /// - [uris]: 1 to [maxPostGetUris] post AT-URIs (throws [ArgumentError] /// otherwise) - Future> getPosts({required List uris}) { + // Kept `async` (like the other validating methods below) so validation + // failures surface as failed Futures, not synchronous throws — callers + // using .catchError or Future.wait must observe them asynchronously. + Future> getPosts({required List uris}) async { if (uris.isEmpty) { throw ArgumentError.value(uris, 'uris', 'must not be empty'); } @@ -701,7 +707,7 @@ class CovesApiService { required String didLabel, required String endpoint, required String dataKey, - }) { + }) async { if (did.isEmpty || !did.startsWith('did:')) { throw ApiException('Invalid $didLabel DID'); } @@ -726,7 +732,7 @@ class CovesApiService { required String didLabel, required String endpoint, required String dataKey, - }) { + }) async { if (did.isEmpty || !did.startsWith('did:')) { throw ApiException('Invalid $didLabel DID'); } @@ -758,7 +764,7 @@ class CovesApiService { required String communityDid, required Uint8List imageBytes, required String mimeType, - }) { + }) async { // Validate image size (max 1 MB) const maxSizeBytes = 1024 * 1024; // 1 MB if (imageBytes.length > maxSizeBytes) { @@ -810,7 +816,7 @@ class CovesApiService { String? avatarMimeType, Uint8List? bannerBytes, String? bannerMimeType, - }) { + }) async { // Validate avatar if provided if (avatarBytes != null) { if (avatarMimeType == null) { @@ -893,7 +899,7 @@ class CovesApiService { required String targetUri, required String reason, String? explanation, - }) { + }) async { // Validate inputs before making API call const validReasons = { 'spam', diff --git a/lib/services/log_redaction.dart b/lib/services/log_redaction.dart index 5ca6521..45431cf 100644 --- a/lib/services/log_redaction.dart +++ b/lib/services/log_redaction.dart @@ -1,16 +1,35 @@ -/// Bearer-token redaction for debug logs. +/// Credential redaction for debug logs. /// /// Shared by every service that prints request/response data — credentials -/// must never reach the logs, even in debug builds. +/// must never reach the logs, even in debug builds. Covers two shapes: +/// Authorization-header text (`Bearer `) and token-like key/value +/// fields in JSON or Map.toString() output (`access_token: ...`, +/// `"refreshJwt": "..."`). Anything else (e.g. a bare token with no key +/// next to it) is NOT caught — don't log raw bodies from new token-bearing +/// endpoints on the strength of this function alone. library; /// 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. -final RegExp _bearerTokenPattern = RegExp(r'Bearer\s+\S+', caseSensitive: false); +final RegExp _bearerTokenPattern = RegExp( + r'Bearer\s+\S+', + caseSensitive: false, +); -/// Replaces bearer token values with a placeholder so credentials never -/// appear in logs. +/// Matches token-like fields in JSON bodies (`"access_token": "..."`) and +/// Dart Map.toString() output (`sealed_token: ...`). The key must contain +/// `token` or `jwt`; the value run stops at whitespace, commas, or braces +/// so surrounding structure survives. +final RegExp _tokenFieldPattern = RegExp( + '(["\']?[a-z_-]*(?:token|jwt)[a-z_-]*["\']?\\s*[:=]\\s*)["\']?[^"\'\\s,}]+["\']?', + caseSensitive: false, +); + +/// Replaces bearer tokens and token-like field values with a placeholder +/// so credentials never appear in logs. String redactBearerTokens(String line) { - return line.replaceAll(_bearerTokenPattern, 'Bearer [REDACTED]'); + return line + .replaceAll(_bearerTokenPattern, 'Bearer [REDACTED]') + .replaceAllMapped(_tokenFieldPattern, (m) => '${m[1]}[REDACTED]'); } diff --git a/test/services/api_exceptions_test.dart b/test/services/api_exceptions_test.dart index a145833..4d04a97 100644 --- a/test/services/api_exceptions_test.dart +++ b/test/services/api_exceptions_test.dart @@ -1,11 +1,16 @@ +import 'dart:io'; + import 'package:coves_flutter/services/api_exceptions.dart'; import 'package:dio/dio.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter_test/flutter_test.dart'; /// Tests for the canonical DioException → ApiException mapper. /// /// This is the single mapping used by CovesApiService, VoteService, and -/// CommentService — behavior asserted here holds for every endpoint. +/// CommentService — behavior asserted here holds for every endpoint that +/// delegates to it (CommentService.deleteComment substitutes its own +/// message copy for 403/404 but keeps the taxonomy). void main() { DioException responseError( int statusCode, @@ -109,6 +114,37 @@ void main() { expect(exception.message, 'Request failed with status 400'); }); + test('does not throw on non-string message/error fields', () { + // A hostile or buggy server can return structured values here; the + // mapper must never let a TypeError escape the taxonomy. + final exception = ApiException.fromDioError( + responseError(400, { + 'message': {'detail': 'structured'}, + 'error': 400, + }), + ); + + expect(exception, isA()); + expect(exception.message, 'Request failed with status 400'); + }); + + test('treats empty-string message as absent', () { + final exception = ApiException.fromDioError( + responseError(401, {'message': '', 'error': 'ExpiredToken'}), + ); + + expect(exception, isA()); + expect(exception.message, 'ExpiredToken'); + }); + + test('uses the default message for non-Map, non-String bodies', () { + final exception = ApiException.fromDioError( + responseError(400, ['unexpected', 'list']), + ); + + expect(exception.message, 'Request failed with status 400'); + }); + test('maps by status code regardless of DioExceptionType', () { final exception = ApiException.fromDioError( responseError( @@ -156,6 +192,20 @@ void main() { expect(exception, isA()); }); + test('detects DNS failure via the typed SocketException too', () { + // Dio's message text varies by platform; the typed inner error is + // the reliable signal. + final exception = ApiException.fromDioError( + DioException( + requestOptions: RequestOptions(path: '/test'), + type: DioExceptionType.connectionError, + error: const SocketException("Failed host lookup: 'x.example'"), + ), + ); + + expect(exception, isA()); + }); + test('maps bad certificates to NetworkException', () { final exception = ApiException.fromDioError( networkError(DioExceptionType.badCertificate), @@ -173,13 +223,85 @@ void main() { expect(exception.message, contains('cancelled')); }); - test('maps unknown errors to NetworkException', () { + test('maps unknown errors wrapping an IOException to NetworkException', + () { final exception = ApiException.fromDioError( - networkError(DioExceptionType.unknown), + DioException( + requestOptions: RequestOptions(path: '/test'), + error: const SocketException('Connection reset by peer'), + ), ); expect(exception, isA()); expect(exception.message, contains('Network error')); }); + + test( + 'maps unknown errors wrapping a FormatException to a parse ' + 'ApiException, not a network error', () { + // A truncated 200 body must not tell the user to check their + // connection. + final exception = ApiException.fromDioError( + DioException( + requestOptions: RequestOptions(path: '/test'), + error: const FormatException('Unexpected end of input'), + ), + ); + + expect(exception, isA()); + expect(exception, isNot(isA())); + expect(exception.message, 'Failed to parse server response'); + }); + + test('maps bare unknown errors to a plain ApiException', () { + final exception = ApiException.fromDioError( + networkError(DioExceptionType.unknown), + ); + + expect(exception, isA()); + expect(exception, isNot(isA())); + expect(exception.message, contains('Unknown error')); + }); + }); + + group('mapDioException', () { + late List logLines; + late DebugPrintCallback originalDebugPrint; + + setUp(() { + logLines = []; + originalDebugPrint = debugPrint; + debugPrint = (String? message, {int? wrapWidth}) { + logLines.add(message ?? ''); + }; + }); + + tearDown(() { + debugPrint = originalDebugPrint; + }); + + test('returns the same mapping as fromDioError', () { + final error = responseError(404, {'message': 'gone'}); + + final exception = mapDioException(error, operation: 'test op'); + + expect(exception, isA()); + expect(exception.message, 'gone'); + }); + + test('redacts tokens echoed in the logged response body', () { + const token = 'abc!def.secret~token'; + final error = responseError(500, { + 'error': 'InternalServerError', + 'message': 'debug echo: Bearer $token from upstream', + }); + + mapDioException(error, operation: 'test op'); + + final output = logLines.join('\n'); + expect(output, contains('Data:')); + expect(output, contains('Bearer [REDACTED]')); + expect(output, isNot(contains(token))); + }); }); } diff --git a/test/services/auth_interceptor_test.dart b/test/services/auth_interceptor_test.dart new file mode 100644 index 0000000..638719f --- /dev/null +++ b/test/services/auth_interceptor_test.dart @@ -0,0 +1,92 @@ +import 'package:coves_flutter/services/auth_interceptor.dart'; +import 'package:dio/dio.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http_mock_adapter/http_mock_adapter.dart'; + +/// Direct tests for the shared 401-refresh interceptor. +/// +/// The per-service token-refresh tests cover the happy retry paths through +/// real services; this file pins the branches those tests can't reach. +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + group('createAuthInterceptor', () { + late Dio dio; + late DioAdapter dioAdapter; + late int refreshCount; + late int signOutCount; + + setUp(() { + dio = Dio(BaseOptions(baseUrl: 'https://api.test.coves.social')); + dioAdapter = DioAdapter(dio: dio); + refreshCount = 0; + signOutCount = 0; + }); + + void addInterceptor({Future Function()? refresher}) { + dio.interceptors.add( + createAuthInterceptor( + tokenGetter: () async => 'token-1', + tokenRefresher: refresher ?? + () async { + refreshCount++; + return false; + }, + signOutHandler: () async { + signOutCount++; + }, + serviceName: 'TestService', + dio: dio, + ), + ); + } + + test( + '401 from the refresh endpoint signs out without attempting ' + 'a refresh (no infinite loop)', () async { + addInterceptor(); + dioAdapter.onPost( + '/oauth/refresh', + (server) => server.reply(401, {'error': 'Unauthorized'}), + ); + + await expectLater( + dio.post>('/oauth/refresh'), + throwsA(isA()), + ); + + expect(refreshCount, 0); + expect(signOutCount, 1); + }); + + test( + 'an Error thrown by the refresher propagates the original 401 ' + 'without signing out', () async { + // The interceptor catches Object so a TypeError/StateError from the + // auth provider cannot escape the async onError handler (which would + // surface it as a misclassified network error). It must NOT sign out + // either: the refresher owns that decision, and an internal error is + // not evidence the session is dead. + addInterceptor( + refresher: () async => throw StateError('auth provider broken'), + ); + dioAdapter.onGet( + '/xrpc/test.endpoint', + (server) => server.reply(401, {'error': 'Unauthorized'}), + ); + + await expectLater( + dio.get>('/xrpc/test.endpoint'), + throwsA( + isA().having( + (e) => e.response?.statusCode, + 'statusCode', + 401, + ), + ), + ); + + expect(signOutCount, 0); + }); + }); +} diff --git a/test/services/comment_service_test.dart b/test/services/comment_service_test.dart index de03734..1527001 100644 --- a/test/services/comment_service_test.dart +++ b/test/services/comment_service_test.dart @@ -178,6 +178,39 @@ void main() { ); }); + test('should throw NotFoundException on 404 response', () async { + when( + mockDio.post>( + '/xrpc/social.coves.community.comment.create', + data: anyNamed('data'), + ), + ).thenThrow( + DioException( + requestOptions: RequestOptions(path: ''), + type: DioExceptionType.badResponse, + response: Response( + requestOptions: RequestOptions(path: ''), + statusCode: 404, + data: {'message': 'Post not found'}, + ), + ), + ); + + expect( + () => commentService.createComment( + rootUri: 'at://did:plc:author/post/123', + rootCid: 'rootCid', + parentUri: 'at://did:plc:author/post/123', + parentCid: 'parentCid', + content: 'Test comment', + ), + throwsA( + isA() + .having((e) => e.message, 'message', 'Post not found'), + ), + ); + }); + test( 'should throw ApiException on invalid response (null data)', () async { @@ -362,5 +395,80 @@ void main() { ).called(1); }); }); + + group('deleteComment', () { + late MockDio mockDio; + late CommentService commentService; + + const commentUri = 'at://did:plc:test/social.coves.community.comment/1'; + + setUp(() { + mockDio = MockDio(); + when(mockDio.interceptors).thenReturn(Interceptors()); + commentService = CommentService( + sessionGetter: () async => CovesSession( + token: 'test-token', + did: 'did:plc:test', + sessionId: 'test-session-id', + handle: 'test.user', + ), + tokenRefresher: () async => true, + signOutHandler: () async {}, + dio: mockDio, + ); + }); + + DioException statusError(int statusCode) => DioException( + requestOptions: RequestOptions(path: ''), + type: DioExceptionType.badResponse, + response: Response( + requestOptions: RequestOptions(path: ''), + statusCode: statusCode, + data: {'error': 'Forbidden'}, + ), + ); + + test('403 keeps the friendly not-your-comment copy', () async { + when( + mockDio.post>( + '/xrpc/social.coves.community.comment.delete', + data: anyNamed('data'), + ), + ).thenThrow(statusError(403)); + + expect( + () => commentService.deleteComment(uri: commentUri), + throwsA( + isA() + .having((e) => e.statusCode, 'statusCode', 403) + .having( + (e) => e.message, + 'message', + 'You can only delete your own comments', + ), + ), + ); + }); + + test('404 keeps the friendly already-deleted copy', () async { + when( + mockDio.post>( + '/xrpc/social.coves.community.comment.delete', + data: anyNamed('data'), + ), + ).thenThrow(statusError(404)); + + expect( + () => commentService.deleteComment(uri: commentUri), + throwsA( + isA().having( + (e) => e.message, + 'message', + 'Comment not found. It may have already been deleted.', + ), + ), + ); + }); + }); }); } diff --git a/test/services/coves_api_service_redaction_test.dart b/test/services/coves_api_service_redaction_test.dart index d500309..7547cfc 100644 --- a/test/services/coves_api_service_redaction_test.dart +++ b/test/services/coves_api_service_redaction_test.dart @@ -112,5 +112,26 @@ void main() { 'Bearer [REDACTED] and Bearer [REDACTED]', ); }); + + test('redacts token-like JSON fields without a Bearer prefix', () { + expect( + redactBearerTokens('{"access_token": "abc!def.123", "ok": 1}'), + '{"access_token": [REDACTED], "ok": 1}', + ); + }); + + test('redacts token-like fields in Map.toString() output', () { + expect( + redactBearerTokens('{sealed_token: abc.def~x, accessJwt: eyJx.y.z}'), + '{sealed_token: [REDACTED], accessJwt: [REDACTED]}', + ); + }); + + test('leaves non-credential fields untouched', () { + expect( + redactBearerTokens('{cursor: abc123, limit: 50}'), + '{cursor: abc123, limit: 50}', + ); + }); }); } diff --git a/test/services/coves_api_service_token_refresh_test.dart b/test/services/coves_api_service_token_refresh_test.dart index 0c780d3..9a815bc 100644 --- a/test/services/coves_api_service_token_refresh_test.dart +++ b/test/services/coves_api_service_token_refresh_test.dart @@ -143,48 +143,10 @@ void main() { expect(signOutCallCount, 0); }); - test( - 'should NOT retry refresh endpoint on 401 (avoid infinite loop)', - () async { - // This test verifies that the interceptor checks for /oauth/refresh - // in the path to avoid infinite loops. Due to limitations with mocking - // complex request/response cycles, we test this by verifying the - // refresher runs exactly once and the error propagates. - - // Set refresh to fail (simulates refresh endpoint returning 401) - shouldRefreshSucceed = false; - - const postUri = 'at://did:plc:test/social.coves.post.record/123'; - - dioAdapter.onGet( - '/xrpc/social.coves.community.comment.getComments', - (server) => server.reply(401, { - 'error': 'Unauthorized', - 'message': 'Token expired', - }), - queryParameters: { - 'post': postUri, - 'sort': 'hot', - 'depth': 10, - 'limit': 50, - }, - ); - - // Make the request and expect it to fail - expect( - () => apiService.getComments(postUri: postUri), - throwsA(isA()), - ); - - // Wait for async operations to complete - await Future.delayed(const Duration(milliseconds: 100)); - - // The refresher ran exactly once (no infinite loop), and the - // interceptor left the sign-out decision to it - expect(tokenRefreshCallCount, 1); - expect(signOutCallCount, 0); - }, - ); + // The refresh-endpoint guard (401 from /oauth/refresh must not trigger + // another refresh) is tested directly in auth_interceptor_test.dart — + // a previous test here claimed to cover it but only duplicated the + // refresh-failure scenario above without driving that path. test( 'should NOT sign out when token refresh throws exception', diff --git a/test/services/vote_service_test.dart b/test/services/vote_service_test.dart index b0fa341..3e3ced4 100644 --- a/test/services/vote_service_test.dart +++ b/test/services/vote_service_test.dart @@ -1,7 +1,13 @@ +import 'package:coves_flutter/models/coves_session.dart'; +import 'package:coves_flutter/services/api_exceptions.dart'; import 'package:coves_flutter/services/vote_service.dart'; +import 'package:dio/dio.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:http_mock_adapter/http_mock_adapter.dart'; void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + group('VoteService', () { group('VoteResponse', () { test('should create response with uri, cid, and rkey', () { @@ -29,6 +35,41 @@ void main() { }); // DioException → ApiException mapping is covered by - // api_exceptions_test.dart — VoteService delegates to the shared mapper. + // api_exceptions_test.dart — this test just seals the delegation. + group('createVote error mapping', () { + test('surfaces a 500 as ServerException via the canonical mapper', + () async { + final dio = Dio(BaseOptions(baseUrl: 'https://api.test.coves.social')); + final dioAdapter = DioAdapter(dio: dio); + final service = VoteService( + sessionGetter: () async => const CovesSession( + token: 'test-token', + did: 'did:plc:test', + sessionId: 'session-1', + ), + didGetter: () => 'did:plc:test', + dio: dio, + ); + + dioAdapter.onPost( + '/xrpc/social.coves.feed.vote.create', + (server) => server.reply(500, {'message': 'boom'}), + data: { + 'subject': {'uri': 'at://did:plc:test/post/1', 'cid': 'cid1'}, + 'direction': 'up', + }, + ); + + await expectLater( + service.createVote( + postUri: 'at://did:plc:test/post/1', + postCid: 'cid1', + ), + throwsA( + isA().having((e) => e.message, 'message', 'boom'), + ), + ); + }); + }); }); }