Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
60e3bb16ba |
@@ -1,59 +0,0 @@
|
||||
# Implementation Plan: Secure WebView for HTML Emails (#21)
|
||||
|
||||
## Goal
|
||||
Replace the current `flutter_html` based rendering with a hardened WebView-based approach to improve rendering fidelity while strictly enforcing security and privacy.
|
||||
|
||||
## 1. Dependency Management
|
||||
- **Core**: `webview_flutter` (v4+)
|
||||
- **Linux Platform**: `webview_flutter_linux` (Official community-supported or WebKitGTK based implementation). *Note: I will verify the exact package name during implementation.*
|
||||
- **Utilities**: `url_launcher` (existing) for opening links in the system browser.
|
||||
|
||||
## 2. Secure WebView Component (`lib/ui/widgets/secure_email_webview.dart`)
|
||||
Create a new widget `SecureEmailWebView` that encapsulates the `WebViewWidget` and its controller.
|
||||
|
||||
### Configuration & Hardening
|
||||
- **Disable JavaScript**: `controller.setJavaScriptMode(JavaScriptMode.disabled)`.
|
||||
- **Background**: Match the application theme (e.g., transparent or surface color).
|
||||
- **Security Headers/CSP**: Inject a Content Security Policy via `<meta>` tag in the HTML wrapper:
|
||||
- `default-src 'none'; style-src 'unsafe-inline'; img-src 'self' data:;` (Blocks all external assets by default).
|
||||
|
||||
### Image Blocking Logic
|
||||
- **Initial State**: Block remote images by injecting a CSP that restricts `img-src` to `data:` and local schemes.
|
||||
- **Toggle Mechanism**:
|
||||
- Provide a "Load Remote Images" button in the Flutter UI.
|
||||
- When triggered, re-render the HTML with an updated CSP: `img-src * data:;`.
|
||||
|
||||
### Link Interception & Phishing Protection
|
||||
- Implement `NavigationDelegate.onNavigationRequest`.
|
||||
- **Process**:
|
||||
1. Intercept any URL that doesn't start with `about:blank` or `data:`.
|
||||
2. Block the navigation in the WebView.
|
||||
3. Trigger a Flutter `showDialog` for confirmation.
|
||||
- **Phishing Protection Dialog**:
|
||||
- Show the full URL.
|
||||
- **Bold the FQDN**: Parse the URL using `Uri.parse`.
|
||||
- Example: `https://`**`important-bank.com`**`/login`
|
||||
- "Open in Browser" button uses `url_launcher`.
|
||||
|
||||
## 3. Integration Plan
|
||||
### Step 1: Initialization
|
||||
Modify `lib/main.dart` to initialize the Linux WebView platform (using `webview_flutter_linux` or similar) during app startup.
|
||||
|
||||
### Step 2: Replace Renderer in Screens
|
||||
- **EmailDetailScreen**: Replace `Html(...)` with `SecureEmailWebView(html: body.htmlBody!)`.
|
||||
- **ThreadDetailScreen**: Replace `Html(...)` with `SecureEmailWebView(html: body.htmlBody!)`.
|
||||
- Remove `flutter_html` imports and dependencies once migration is complete.
|
||||
|
||||
## 4. Verification & Security Audit
|
||||
- **Manual Tests**:
|
||||
- Open emails with complex HTML layouts.
|
||||
- Verify images are blocked initially.
|
||||
- Verify "Load images" works.
|
||||
- Click various links (http, https, mailto) and verify the confirmation dialog and FQDN bolding.
|
||||
- **Security Check**:
|
||||
- Verify that `<script>` tags are not executed.
|
||||
- Verify no network requests for external images occur before user consent (via DevTools or proxy).
|
||||
|
||||
## 5. Potential Challenges
|
||||
- **Linux WebView Stability**: WebKitGTK on Linux can sometimes have rendering or sizing issues in Flutter.
|
||||
- **Scrolling**: Ensuring the WebView integrates smoothly into the `ListView` of the email detail screen (might require fixed height or `SizedBox`).
|
||||
@@ -1,19 +1,14 @@
|
||||
import 'package:sharedinbox/core/models/email.dart';
|
||||
|
||||
abstract class EmailRepository {
|
||||
Stream<List<Email>> observeEmails(
|
||||
String accountId,
|
||||
String mailboxPath, {
|
||||
int limit = 50,
|
||||
});
|
||||
Stream<List<Email>> observeEmails(String accountId, String mailboxPath);
|
||||
|
||||
/// Groups emails by threadId and returns one [EmailThread] per thread,
|
||||
/// sorted by the latest message date descending.
|
||||
Stream<List<EmailThread>> observeThreads(
|
||||
String accountId,
|
||||
String mailboxPath, {
|
||||
int limit = 50,
|
||||
});
|
||||
String mailboxPath,
|
||||
);
|
||||
|
||||
/// Returns all emails belonging to [threadId] in [mailboxPath].
|
||||
Stream<List<Email>> observeEmailsInThread(
|
||||
|
||||
@@ -269,47 +269,10 @@ class AppDatabase extends _$AppDatabase {
|
||||
AppDatabase([QueryExecutor? executor]) : super(executor ?? _openConnection());
|
||||
|
||||
@override
|
||||
int get schemaVersion => 26;
|
||||
|
||||
Future<void> _createEmailFts() async {
|
||||
await customStatement('''
|
||||
CREATE VIRTUAL TABLE IF NOT EXISTS email_fts USING fts5(
|
||||
subject, preview, from_json,
|
||||
content='emails',
|
||||
content_rowid='rowid'
|
||||
)
|
||||
''');
|
||||
await customStatement('''
|
||||
CREATE TRIGGER IF NOT EXISTS email_fts_ai
|
||||
AFTER INSERT ON emails BEGIN
|
||||
INSERT INTO email_fts(rowid, subject, preview, from_json)
|
||||
VALUES (new.rowid, new.subject, new.preview, new.from_json);
|
||||
END
|
||||
''');
|
||||
await customStatement('''
|
||||
CREATE TRIGGER IF NOT EXISTS email_fts_au
|
||||
AFTER UPDATE OF subject, preview, from_json ON emails BEGIN
|
||||
INSERT INTO email_fts(email_fts, rowid, subject, preview, from_json)
|
||||
VALUES ('delete', old.rowid, old.subject, old.preview, old.from_json);
|
||||
INSERT INTO email_fts(rowid, subject, preview, from_json)
|
||||
VALUES (new.rowid, new.subject, new.preview, new.from_json);
|
||||
END
|
||||
''');
|
||||
await customStatement('''
|
||||
CREATE TRIGGER IF NOT EXISTS email_fts_ad
|
||||
AFTER DELETE ON emails BEGIN
|
||||
INSERT INTO email_fts(email_fts, rowid, subject, preview, from_json)
|
||||
VALUES ('delete', old.rowid, old.subject, old.preview, old.from_json);
|
||||
END
|
||||
''');
|
||||
}
|
||||
int get schemaVersion => 25;
|
||||
|
||||
@override
|
||||
MigrationStrategy get migration => MigrationStrategy(
|
||||
onCreate: (m) async {
|
||||
await m.createAll();
|
||||
await _createEmailFts();
|
||||
},
|
||||
onUpgrade: (m, from, to) async {
|
||||
// NOTE: m.createTable(T) creates the LATEST version of table T.
|
||||
// If you later add a column C to T in version X, you must guard
|
||||
@@ -484,14 +447,6 @@ class AppDatabase extends _$AppDatabase {
|
||||
),
|
||||
);
|
||||
}
|
||||
if (from < 26) {
|
||||
await _createEmailFts();
|
||||
// Backfill FTS index from existing rows.
|
||||
await customStatement('''
|
||||
INSERT INTO email_fts(rowid, subject, preview, from_json)
|
||||
SELECT rowid, subject, preview, from_json FROM emails
|
||||
''');
|
||||
}
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
@@ -58,17 +58,15 @@ class EmailRepositoryImpl implements EmailRepository {
|
||||
@override
|
||||
Stream<List<model.Email>> observeEmails(
|
||||
String accountId,
|
||||
String mailboxPath, {
|
||||
int limit = 50,
|
||||
}) {
|
||||
String mailboxPath,
|
||||
) {
|
||||
return (_db.select(_db.emails)
|
||||
..where(
|
||||
(t) =>
|
||||
t.accountId.equals(accountId) &
|
||||
t.mailboxPath.equals(mailboxPath),
|
||||
)
|
||||
..orderBy([(t) => OrderingTerm.desc(t.receivedAt)])
|
||||
..limit(limit))
|
||||
..orderBy([(t) => OrderingTerm.desc(t.receivedAt)]))
|
||||
.watch()
|
||||
.map((rows) => rows.map(_toModel).toList());
|
||||
}
|
||||
@@ -76,17 +74,15 @@ class EmailRepositoryImpl implements EmailRepository {
|
||||
@override
|
||||
Stream<List<model.EmailThread>> observeThreads(
|
||||
String accountId,
|
||||
String mailboxPath, {
|
||||
int limit = 50,
|
||||
}) {
|
||||
String mailboxPath,
|
||||
) {
|
||||
return (_db.select(_db.threads)
|
||||
..where(
|
||||
(t) =>
|
||||
t.accountId.equals(accountId) &
|
||||
t.mailboxPath.equals(mailboxPath),
|
||||
)
|
||||
..orderBy([(t) => OrderingTerm.desc(t.latestDate)])
|
||||
..limit(limit))
|
||||
..orderBy([(t) => OrderingTerm.desc(t.latestDate)]))
|
||||
.watch()
|
||||
.map((rows) => rows.map(_threadRowToModel).toList());
|
||||
}
|
||||
@@ -2474,39 +2470,28 @@ class EmailRepositoryImpl implements EmailRepository {
|
||||
String? accountId,
|
||||
String query,
|
||||
) async {
|
||||
final ftsQuery = _toFtsQuery(query);
|
||||
if (ftsQuery.isEmpty) return [];
|
||||
|
||||
final sql = accountId != null
|
||||
? 'SELECT e.* FROM email_fts f JOIN emails e ON e.rowid = f.rowid'
|
||||
' WHERE email_fts MATCH ? AND e.account_id = ? ORDER BY rank LIMIT 50'
|
||||
: 'SELECT e.* FROM email_fts f JOIN emails e ON e.rowid = f.rowid'
|
||||
' WHERE email_fts MATCH ? ORDER BY rank LIMIT 50';
|
||||
final variables = accountId != null
|
||||
? [Variable<String>(ftsQuery), Variable<String>(accountId)]
|
||||
: [Variable<String>(ftsQuery)];
|
||||
|
||||
final queryRows = await _db
|
||||
.customSelect(sql, variables: variables, readsFrom: {_db.emails}).get();
|
||||
final emailRows = await Future.wait(
|
||||
queryRows.map((r) => _db.emails.mapFromRow(r)),
|
||||
);
|
||||
return emailRows.map(_toModel).toList();
|
||||
}
|
||||
|
||||
/// Converts a user query string into an FTS5 match expression.
|
||||
/// Each whitespace-separated word becomes a prefix term (word*) so that
|
||||
/// partial words still match. Special FTS5 characters are stripped.
|
||||
static String _toFtsQuery(String query) {
|
||||
final words = query
|
||||
.trim()
|
||||
.toLowerCase()
|
||||
.split(RegExp(r'\s+'))
|
||||
.where((w) => w.isNotEmpty)
|
||||
.map((w) => w.replaceAll(RegExp(r'[^\w]'), ''))
|
||||
.where((w) => w.isNotEmpty)
|
||||
.toList();
|
||||
if (words.isEmpty) return '';
|
||||
return words.map((w) => '$w*').join(' ');
|
||||
final rows = await (_db.select(_db.emails)
|
||||
..where((t) {
|
||||
Expression<bool> condition = const Constant(true);
|
||||
if (accountId != null) {
|
||||
condition = t.accountId.equals(accountId);
|
||||
}
|
||||
for (final word in words) {
|
||||
final pattern = '%$word%';
|
||||
condition = condition &
|
||||
(t.subject.like(pattern) | t.preview.like(pattern));
|
||||
}
|
||||
return condition;
|
||||
})
|
||||
..orderBy([(t) => OrderingTerm.desc(t.receivedAt)])
|
||||
..limit(50))
|
||||
.get();
|
||||
return rows.map(_toModel).toList();
|
||||
}
|
||||
|
||||
@override
|
||||
|
||||
+1
-23
@@ -3,7 +3,6 @@ import 'dart:async';
|
||||
import 'package:flutter_riverpod/flutter_riverpod.dart';
|
||||
import 'package:http/http.dart' as http;
|
||||
import 'package:sharedinbox/core/models/account.dart' as model;
|
||||
import 'package:sharedinbox/core/models/email.dart';
|
||||
import 'package:sharedinbox/core/models/undo_action.dart';
|
||||
import 'package:sharedinbox/core/repositories/account_repository.dart';
|
||||
import 'package:sharedinbox/core/repositories/draft_repository.dart';
|
||||
@@ -18,7 +17,7 @@ import 'package:sharedinbox/core/services/undo_service.dart';
|
||||
import 'package:sharedinbox/core/storage/secure_storage.dart';
|
||||
import 'package:sharedinbox/core/sync/account_sync_manager.dart';
|
||||
import 'package:sharedinbox/core/sync/reliability_runner.dart';
|
||||
import 'package:sharedinbox/data/db/database.dart' hide Email, EmailBody;
|
||||
import 'package:sharedinbox/data/db/database.dart';
|
||||
import 'package:sharedinbox/data/imap/imap_client_factory.dart';
|
||||
import 'package:sharedinbox/data/jmap/sieve_repository.dart';
|
||||
import 'package:sharedinbox/data/repositories/account_repository_impl.dart';
|
||||
@@ -169,27 +168,6 @@ final undoServiceProvider =
|
||||
return service;
|
||||
});
|
||||
|
||||
/// Loads email header + body and marks the email as seen.
|
||||
/// Owned by [EmailDetailScreen]; decouples data loading from the widget tree.
|
||||
final emailDetailProvider = AsyncNotifierProvider.autoDispose
|
||||
.family<EmailDetailNotifier, (Email?, EmailBody), String>(
|
||||
EmailDetailNotifier.new,
|
||||
);
|
||||
|
||||
class EmailDetailNotifier
|
||||
extends AutoDisposeFamilyAsyncNotifier<(Email?, EmailBody), String> {
|
||||
@override
|
||||
Future<(Email?, EmailBody)> build(String emailId) async {
|
||||
final repo = ref.read(emailRepositoryProvider);
|
||||
final results = await Future.wait([
|
||||
repo.getEmail(emailId),
|
||||
repo.getEmailBody(emailId),
|
||||
]);
|
||||
unawaited(repo.setFlag(emailId, seen: true));
|
||||
return (results[0] as Email?, results[1] as EmailBody);
|
||||
}
|
||||
}
|
||||
|
||||
final accountByIdProvider =
|
||||
StreamProvider.autoDispose.family<model.Account?, String>((ref, accountId) {
|
||||
return ref.watch(accountRepositoryProvider).observeAccounts().map(
|
||||
|
||||
@@ -26,130 +26,144 @@ class EmailDetailScreen extends ConsumerStatefulWidget {
|
||||
}
|
||||
|
||||
class _EmailDetailScreenState extends ConsumerState<EmailDetailScreen> {
|
||||
late final Future<(Email?, EmailBody)> _dataFuture;
|
||||
bool _isFlagged = false;
|
||||
bool _loadRemoteImages = false;
|
||||
final Set<String> _downloading = {};
|
||||
|
||||
@override
|
||||
void initState() {
|
||||
super.initState();
|
||||
final repo = ref.read(emailRepositoryProvider);
|
||||
_dataFuture = Future.wait([
|
||||
repo.getEmail(widget.emailId),
|
||||
repo.getEmailBody(widget.emailId),
|
||||
]).then((results) {
|
||||
final email = results[0] as Email?;
|
||||
if (email != null && mounted) {
|
||||
setState(() => _isFlagged = email.isFlagged);
|
||||
}
|
||||
return (email, results[1] as EmailBody);
|
||||
});
|
||||
unawaited(repo.setFlag(widget.emailId, seen: true));
|
||||
}
|
||||
|
||||
@override
|
||||
Widget build(BuildContext context) {
|
||||
final repo = ref.watch(emailRepositoryProvider);
|
||||
final detail = ref.watch(emailDetailProvider(widget.emailId));
|
||||
return FutureBuilder<(Email?, EmailBody)>(
|
||||
future: _dataFuture,
|
||||
builder: (ctx, snap) {
|
||||
final header = snap.data?.$1;
|
||||
final body = snap.data?.$2;
|
||||
|
||||
ref.listen<AsyncValue<(Email?, EmailBody)>>(
|
||||
emailDetailProvider(widget.emailId),
|
||||
(_, next) {
|
||||
final email = next.valueOrNull?.$1;
|
||||
if (email != null && mounted) {
|
||||
setState(() => _isFlagged = email.isFlagged);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
final header = detail.valueOrNull?.$1;
|
||||
final body = detail.valueOrNull?.$2;
|
||||
|
||||
return Scaffold(
|
||||
appBar: AppBar(
|
||||
title: Text(
|
||||
header?.subject ?? '(loading…)',
|
||||
overflow: TextOverflow.ellipsis,
|
||||
),
|
||||
actions: [
|
||||
IconButton(
|
||||
icon: const Icon(Icons.reply),
|
||||
tooltip: 'Reply',
|
||||
onPressed: header == null
|
||||
? null
|
||||
: () => _reply(context, header, body, replyAll: false),
|
||||
),
|
||||
IconButton(
|
||||
icon: const Icon(Icons.reply_all),
|
||||
tooltip: 'Reply all',
|
||||
onPressed: header == null
|
||||
? null
|
||||
: () => _reply(context, header, body, replyAll: true),
|
||||
),
|
||||
IconButton(
|
||||
icon: const Icon(Icons.forward),
|
||||
tooltip: 'Forward',
|
||||
onPressed:
|
||||
header == null ? null : () => _forward(context, header, body),
|
||||
),
|
||||
IconButton(
|
||||
icon: const Icon(Icons.mark_email_unread_outlined),
|
||||
tooltip: 'Mark as unread',
|
||||
onPressed: () async {
|
||||
await repo.setFlag(widget.emailId, seen: false);
|
||||
if (context.mounted) context.pop();
|
||||
},
|
||||
),
|
||||
IconButton(
|
||||
icon: Icon(
|
||||
_isFlagged ? Icons.star : Icons.star_border,
|
||||
color: _isFlagged ? Colors.amber : null,
|
||||
return Scaffold(
|
||||
appBar: AppBar(
|
||||
title: Text(
|
||||
header?.subject ?? '(loading…)',
|
||||
overflow: TextOverflow.ellipsis,
|
||||
),
|
||||
tooltip: _isFlagged ? 'Unflag' : 'Flag',
|
||||
onPressed: () async {
|
||||
final next = !_isFlagged;
|
||||
await repo.setFlag(widget.emailId, flagged: next);
|
||||
if (mounted) setState(() => _isFlagged = next);
|
||||
},
|
||||
),
|
||||
IconButton(
|
||||
icon: const Icon(Icons.drive_file_move_outline),
|
||||
tooltip: 'Move to folder',
|
||||
onPressed: header == null ? null : () => _moveTo(context, header),
|
||||
),
|
||||
IconButton(
|
||||
icon: const Icon(Icons.access_time),
|
||||
tooltip: 'Snooze',
|
||||
onPressed: header == null ? null : () => _snooze(context, header),
|
||||
),
|
||||
IconButton(
|
||||
icon: const Icon(Icons.delete),
|
||||
tooltip: 'Delete',
|
||||
onPressed: () async {
|
||||
final destPath = await repo.deleteEmail(widget.emailId);
|
||||
actions: [
|
||||
IconButton(
|
||||
icon: const Icon(Icons.reply),
|
||||
tooltip: 'Reply',
|
||||
onPressed: header == null
|
||||
? null
|
||||
: () => _reply(context, header, body, replyAll: false),
|
||||
),
|
||||
IconButton(
|
||||
icon: const Icon(Icons.reply_all),
|
||||
tooltip: 'Reply all',
|
||||
onPressed: header == null
|
||||
? null
|
||||
: () => _reply(context, header, body, replyAll: true),
|
||||
),
|
||||
IconButton(
|
||||
icon: const Icon(Icons.forward),
|
||||
tooltip: 'Forward',
|
||||
onPressed: header == null
|
||||
? null
|
||||
: () => _forward(context, header, body),
|
||||
),
|
||||
IconButton(
|
||||
icon: const Icon(Icons.mark_email_unread_outlined),
|
||||
tooltip: 'Mark as unread',
|
||||
onPressed: () async {
|
||||
await repo.setFlag(widget.emailId, seen: false);
|
||||
if (context.mounted) context.pop();
|
||||
},
|
||||
),
|
||||
IconButton(
|
||||
icon: Icon(
|
||||
_isFlagged ? Icons.star : Icons.star_border,
|
||||
color: _isFlagged ? Colors.amber : null,
|
||||
),
|
||||
tooltip: _isFlagged ? 'Unflag' : 'Flag',
|
||||
onPressed: () async {
|
||||
final next = !_isFlagged;
|
||||
await repo.setFlag(widget.emailId, flagged: next);
|
||||
if (mounted) setState(() => _isFlagged = next);
|
||||
},
|
||||
),
|
||||
IconButton(
|
||||
icon: const Icon(Icons.drive_file_move_outline),
|
||||
tooltip: 'Move to folder',
|
||||
onPressed:
|
||||
header == null ? null : () => _moveTo(context, header),
|
||||
),
|
||||
IconButton(
|
||||
icon: const Icon(Icons.access_time),
|
||||
tooltip: 'Snooze',
|
||||
onPressed:
|
||||
header == null ? null : () => _snooze(context, header),
|
||||
),
|
||||
IconButton(
|
||||
icon: const Icon(Icons.delete),
|
||||
tooltip: 'Delete',
|
||||
onPressed: () async {
|
||||
final destPath = await repo.deleteEmail(widget.emailId);
|
||||
|
||||
if (header != null) {
|
||||
unawaited(
|
||||
ref.read(undoServiceProvider.notifier).pushAction(
|
||||
UndoAction(
|
||||
id: DateTime.now().toIso8601String(),
|
||||
accountId: header.accountId,
|
||||
type: UndoType.delete,
|
||||
emailIds: [widget.emailId],
|
||||
sourceMailboxPath: header.mailboxPath,
|
||||
destinationMailboxPath: destPath,
|
||||
originalEmails: [header],
|
||||
),
|
||||
),
|
||||
);
|
||||
}
|
||||
if (header != null) {
|
||||
unawaited(
|
||||
ref.read(undoServiceProvider.notifier).pushAction(
|
||||
UndoAction(
|
||||
id: DateTime.now().toIso8601String(),
|
||||
accountId: header.accountId,
|
||||
type: UndoType.delete,
|
||||
emailIds: [widget.emailId],
|
||||
sourceMailboxPath: header.mailboxPath,
|
||||
destinationMailboxPath: destPath,
|
||||
originalEmails: [header],
|
||||
),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
if (context.mounted) context.pop();
|
||||
},
|
||||
),
|
||||
PopupMenuButton<String>(
|
||||
itemBuilder: (ctx) => [
|
||||
const PopupMenuItem(
|
||||
value: 'headers',
|
||||
child: Text('Show Mail Headers'),
|
||||
if (context.mounted) context.pop();
|
||||
},
|
||||
),
|
||||
PopupMenuButton<String>(
|
||||
itemBuilder: (ctx) => [
|
||||
const PopupMenuItem(
|
||||
value: 'headers',
|
||||
child: Text('Show Mail Headers'),
|
||||
),
|
||||
],
|
||||
onSelected: (value) {
|
||||
if (value == 'headers' && body != null) {
|
||||
_showHeaders(context, body);
|
||||
}
|
||||
},
|
||||
),
|
||||
],
|
||||
onSelected: (value) {
|
||||
if (value == 'headers' && body != null) {
|
||||
_showHeaders(context, body);
|
||||
}
|
||||
},
|
||||
),
|
||||
],
|
||||
),
|
||||
body: detail.when(
|
||||
loading: () => const Center(child: CircularProgressIndicator()),
|
||||
error: (e, _) => Center(child: Text('Error: $e')),
|
||||
data: (d) => _buildBody(context, d.$1, d.$2),
|
||||
),
|
||||
body: snap.connectionState == ConnectionState.waiting
|
||||
? const Center(child: CircularProgressIndicator())
|
||||
: snap.hasError
|
||||
? Center(child: Text('Error: ${snap.error}'))
|
||||
: _buildBody(ctx, header, body!),
|
||||
);
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -45,10 +45,6 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
|
||||
List<EmailThread> _currentThreads = [];
|
||||
// Individual email selection used in search results.
|
||||
final Set<String> _selectedSearchIds = {};
|
||||
|
||||
// Pagination: number of threads currently requested from the DB.
|
||||
static const _pageSize = 50;
|
||||
int _limit = _pageSize;
|
||||
bool get _selecting =>
|
||||
_selectedThreadIds.isNotEmpty || _selectedSearchIds.isNotEmpty;
|
||||
|
||||
@@ -347,11 +343,7 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
|
||||
await emailRepo.syncEmails(widget.accountId, widget.mailboxPath);
|
||||
},
|
||||
child: StreamBuilder<List<EmailThread>>(
|
||||
stream: emailRepo.observeThreads(
|
||||
widget.accountId,
|
||||
widget.mailboxPath,
|
||||
limit: _limit,
|
||||
),
|
||||
stream: emailRepo.observeThreads(widget.accountId, widget.mailboxPath),
|
||||
builder: (ctx, snap) {
|
||||
if (!snap.hasData) {
|
||||
return const Center(child: CircularProgressIndicator());
|
||||
@@ -547,16 +539,9 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
|
||||
}
|
||||
|
||||
Widget _buildThreadList(List<EmailThread> threads) {
|
||||
final hasMore = threads.length == _limit;
|
||||
return ListView.builder(
|
||||
itemCount: threads.length + (hasMore ? 1 : 0),
|
||||
itemCount: threads.length,
|
||||
itemBuilder: (ctx, i) {
|
||||
if (i == threads.length) {
|
||||
return TextButton(
|
||||
onPressed: () => setState(() => _limit += _pageSize),
|
||||
child: const Text('Load more'),
|
||||
);
|
||||
}
|
||||
final t = threads[i];
|
||||
final isSelected = _selectedThreadIds.contains(t.threadId);
|
||||
final senderNames =
|
||||
|
||||
-161
@@ -1,161 +0,0 @@
|
||||
# SharedInbox — Improvement Plan
|
||||
|
||||
30 tasks across 7 perspectives. Priority markers: 🔴 high · 🟡 medium · 🟢 nice-to-have.
|
||||
|
||||
---
|
||||
|
||||
## Group 1: Performance
|
||||
|
||||
### P1 — Done: https://codeberg.org/guettli/sharedinbox/pulls/41
|
||||
|
||||
### P1 🔴 Replace LIKE-based search with FTS5 virtual table
|
||||
The current `observeEmails` and search queries use `LIKE '%query%'` which becomes a full-table scan at scale.
|
||||
Create an `email_fts` FTS5 virtual table (subject, preview, fromJson) populated via trigger or sync-time insert.
|
||||
Wire `SearchScreen` to query the FTS table instead.
|
||||
Files: `lib/data/db/database.dart`, `lib/data/repositories/email_repository_impl.dart`.
|
||||
|
||||
### P2 🔴 Lazy-load email bodies on scroll (pagination)
|
||||
`observeThreads` and `observeEmails` return the full list with no limit. As the mailbox grows this streams thousands of rows into memory.
|
||||
Add a page-size parameter (e.g. 50) with "load more" support in `EmailListScreen`.
|
||||
The `EmailBodies` table is already separate — never fetch bodies in the list query.
|
||||
Files: `lib/data/repositories/email_repository_impl.dart`, `lib/ui/screens/email_list_screen.dart`.
|
||||
|
||||
### P3 🟡 Defer HTML parsing off the UI thread using an Isolate
|
||||
`flutter_html` parsing blocks the raster thread for large HTML bodies, causing jank when opening email detail.
|
||||
Move the HTML→Widget tree conversion (or at minimum the `html_utils.dart` HTML-to-plain step) into a `compute()` call.
|
||||
Files: `lib/ui/screens/email_detail_screen.dart`, `lib/core/utils/html_utils.dart`.
|
||||
|
||||
### P4 — Done: https://codeberg.org/guettli/sharedinbox/pulls/36
|
||||
|
||||
### P5 🟢 Cache the formatted date strings in EmailListScreen
|
||||
`DateFormat('MMM d').format(...)` is called for every email on every rebuild. Compute and cache these in the model layer or inside the list item widget's `build` method using a static cache map.
|
||||
Files: `lib/ui/screens/email_list_screen.dart`, `lib/core/utils/format_utils.dart`.
|
||||
|
||||
---
|
||||
|
||||
## Group 2: Reliability & Resilience
|
||||
|
||||
### R1 — Done: https://codeberg.org/guettli/sharedinbox/pulls/20
|
||||
|
||||
### R2 — Done: https://codeberg.org/guettli/sharedinbox/pulls/22
|
||||
|
||||
### R3 — Done: https://codeberg.org/guettli/sharedinbox/pulls/35
|
||||
|
||||
### R4 — Done: https://codeberg.org/guettli/sharedinbox/pulls/23
|
||||
|
||||
### R5 🟡 Handle TLS certificate changes gracefully
|
||||
`tls_error.dart` detects TLS errors but they bubble up as generic errors in the sync loop.
|
||||
Detect `TlsError` specifically in `_AccountSync` and show a user-facing dialog offering to re-add the account or trust the new certificate.
|
||||
Files: `lib/data/imap/tls_error.dart`, `lib/core/sync/account_sync_manager.dart`.
|
||||
|
||||
### R6 — Done: https://codeberg.org/guettli/sharedinbox/pulls/24
|
||||
|
||||
---
|
||||
|
||||
## Group 3: Security
|
||||
|
||||
### S1 🔴 Optional SQLCipher encryption for the Drift database
|
||||
Emails cached locally are plaintext. Users on shared or rooted devices are exposed.
|
||||
Add an opt-in "Encrypt local storage" setting using `drift`'s `encrypted` backend (`sqflite_cipher` / `sqlcipher_flutter_libs`).
|
||||
Store the database key in `flutter_secure_storage` (already present).
|
||||
Files: `lib/data/db/database.dart`, `pubspec.yaml`, a new settings toggle.
|
||||
|
||||
### S2 — Done: https://codeberg.org/guettli/sharedinbox/pulls/25
|
||||
|
||||
### S3 🟡 Enforce certificate pinning for known providers (opt-in)
|
||||
Auto-discovered accounts for major providers (Gmail, Fastmail, Proton) could be pinned to their known CA hierarchy.
|
||||
Implement as an opt-in per-account setting; only applies when the account is auto-discovered via `AccountDiscoveryService`.
|
||||
Files: `lib/core/services/account_discovery_service.dart`, `lib/data/imap/imap_client_factory.dart`.
|
||||
|
||||
### S4 🟢 Audit and restrict external link handling in HTML emails
|
||||
`flutter_html` passes `<a href>` clicks to `url_launcher` without a prompt.
|
||||
Before launching, show a confirmation dialog with the destination URL so phishing links are visible.
|
||||
Files: `lib/ui/screens/email_detail_screen.dart`.
|
||||
|
||||
---
|
||||
|
||||
## Group 4: User Experience
|
||||
|
||||
### U1 — Done: https://codeberg.org/guettli/sharedinbox/pulls/26
|
||||
|
||||
### U2 — Done: https://codeberg.org/guettli/sharedinbox/pulls/27
|
||||
|
||||
### U3 🟡 Add "Recent searches" history to SearchScreen
|
||||
The search bar clears on navigation. Store the last 10 search terms in a local DB table and show them as chips below the search field when the field is focused but empty.
|
||||
Files: `lib/ui/screens/search_screen.dart`, `lib/data/db/database.dart`.
|
||||
|
||||
### U4 — Done: https://codeberg.org/guettli/sharedinbox/pulls/28
|
||||
|
||||
### U5 🟡 Accessible swipe actions on email list items
|
||||
Delete and Move are hidden behind long-press or detail-screen menus. Add leading/trailing swipe actions on the `EmailListScreen` tile (archive / delete) matching Material 3 patterns.
|
||||
Files: `lib/ui/screens/email_list_screen.dart`.
|
||||
|
||||
### U6 — Done: https://codeberg.org/guettli/sharedinbox/pulls/29
|
||||
|
||||
### U7 🟢 Onboarding walkthrough for first-time users
|
||||
The app opens directly to an empty account list with only a `+` button. First-time users have no guidance.
|
||||
Add a one-time welcome card or bottom-sheet with the three-step flow: Add account → wait for sync → open inbox.
|
||||
Files: `lib/ui/screens/account_list_screen.dart`.
|
||||
|
||||
### U8 🟢 "Mark all as read" action in mailbox
|
||||
Power users managing high-volume mailboxes need bulk read marking. Add a "Mark all as read" option in the mailbox overflow menu.
|
||||
Files: `lib/ui/screens/email_list_screen.dart`, `lib/core/repositories/email_repository.dart`, `lib/data/repositories/email_repository_impl.dart`.
|
||||
|
||||
---
|
||||
|
||||
## Group 5: Testing
|
||||
|
||||
### T1 — Done: https://codeberg.org/guettli/sharedinbox/pulls/30
|
||||
|
||||
### T2 — Done: https://codeberg.org/guettli/sharedinbox/pulls/31
|
||||
|
||||
### T3 🟡 Contract tests for all Repository interfaces
|
||||
The interfaces in `core/repositories/` have no shared contract test suite. Concrete impls can silently diverge.
|
||||
Add a shared `EmailRepositoryContract` abstract test class; run it against both `EmailRepositoryImpl` and any future mock/fake. Mirror this for `MailboxRepository` and `AccountRepository`.
|
||||
Files: `test/unit/` (new contract test files).
|
||||
|
||||
### T4 — Done: https://codeberg.org/guettli/sharedinbox/pulls/32
|
||||
|
||||
### T5 🟢 Snapshot / golden tests for key email list states
|
||||
The email list has multiple states: loading, empty, normal, selection mode, search active, error banner.
|
||||
Add golden tests using `matchesGoldenFile` for each state so visual regressions surface in CI.
|
||||
Files: `test/widget/email_list_screen_test.dart`.
|
||||
|
||||
---
|
||||
|
||||
## Group 6: Architecture & Code Quality
|
||||
|
||||
### A1 — Done: https://codeberg.org/guettli/sharedinbox/pulls/39
|
||||
|
||||
### A2 — Done: https://codeberg.org/guettli/sharedinbox/pulls/33
|
||||
|
||||
### A3 🟡 Make AccountSyncManager testable without real IMAP connections
|
||||
`AccountSyncManager` accepts `ImapConnectFn` as a dependency but `_JmapAccountSync` constructs its HTTP client internally.
|
||||
Pass an injectable `http.Client` to `_JmapAccountSync` (already done in `EmailRepositoryImpl`; mirror the pattern here).
|
||||
Files: `lib/core/sync/account_sync_manager.dart`, `test/unit/account_sync_manager_test.dart`.
|
||||
|
||||
### A4 🟡 Replace raw JSON strings in DB with structured encoding
|
||||
`fromJson`, `toAddresses`, `ccJson`, `references` are stored as raw JSON strings parsed on every model conversion.
|
||||
Create typed value classes with `fromJson`/`toJson` in `core/models/email.dart` and add a `TypeConverter` in the Drift schema so the DB layer owns the serialisation.
|
||||
Files: `lib/data/db/database.dart`, `lib/core/models/email.dart`, `lib/data/repositories/email_repository_impl.dart`.
|
||||
|
||||
### A5 🟢 Enforce layer boundaries via lint custom rules or barrel imports
|
||||
The `ui/` layer directly imports `data/` concrete classes in several screens (e.g. `drift` types leak through).
|
||||
Add a custom `analysis_options.yaml` rule or a CI lint step that flags any `ui/` import of `data/` (only `core/` interfaces are allowed from UI).
|
||||
Files: `analysis_options.yaml`, CI config.
|
||||
|
||||
---
|
||||
|
||||
## Group 7: Developer Experience
|
||||
|
||||
### D1 🔴 CI matrix for macOS and Windows builds
|
||||
The CI currently tests Linux and Android. The macOS and Windows targets are "scaffolded" and may have accumulated silent breakage.
|
||||
Add `flutter build macos --debug` and `flutter build windows --debug` jobs to the CI workflow with the same failure threshold as Linux.
|
||||
Files: `.github/workflows/ci.yml` (or Codeberg equivalent).
|
||||
|
||||
### D2 — Done: https://codeberg.org/guettli/sharedinbox/pulls/34
|
||||
|
||||
### D3 🟢 Document the sync protocol in a SYNC.md architecture doc
|
||||
`DB-SYNC.md` exists but focuses on the DB schema. The IMAP IDLE loop, exponential backoff, pending-change queue, and undo cancel logic are spread across four files with no single reference.
|
||||
Write `SYNC.md` that describes the full lifecycle of an email action from UI tap to server confirmation.
|
||||
Files: `SYNC.md` (new).
|
||||
@@ -105,19 +105,10 @@ class _FakeEmails implements EmailRepository {
|
||||
final syncCounts = <String, int>{};
|
||||
|
||||
@override
|
||||
Stream<List<Email>> observeEmails(
|
||||
String a,
|
||||
String m, {
|
||||
int limit = 50,
|
||||
}) =>
|
||||
Stream.value([]);
|
||||
Stream<List<Email>> observeEmails(String a, String m) => Stream.value([]);
|
||||
|
||||
@override
|
||||
Stream<List<EmailThread>> observeThreads(
|
||||
String a,
|
||||
String m, {
|
||||
int limit = 50,
|
||||
}) =>
|
||||
Stream<List<EmailThread>> observeThreads(String a, String m) =>
|
||||
Stream.value([]);
|
||||
|
||||
@override
|
||||
|
||||
@@ -34,18 +34,9 @@ void main() {
|
||||
|
||||
class FakeEmailRepository implements EmailRepository {
|
||||
@override
|
||||
Stream<List<Email>> observeEmails(
|
||||
String a,
|
||||
String m, {
|
||||
int limit = 50,
|
||||
}) =>
|
||||
Stream.value([]);
|
||||
Stream<List<Email>> observeEmails(String a, String m) => Stream.value([]);
|
||||
@override
|
||||
Stream<List<EmailThread>> observeThreads(
|
||||
String a,
|
||||
String m, {
|
||||
int limit = 50,
|
||||
}) =>
|
||||
Stream<List<EmailThread>> observeThreads(String a, String m) =>
|
||||
Stream.value([]);
|
||||
@override
|
||||
Stream<List<Email>> observeEmailsInThread(String a, String m, String t) =>
|
||||
|
||||
@@ -215,10 +215,9 @@ class MockEmailRepository extends _i1.Mock implements _i9.EmailRepository {
|
||||
|
||||
@override
|
||||
_i4.Stream<List<_i2.Email>> observeEmails(
|
||||
String accountId,
|
||||
String mailboxPath, {
|
||||
int limit = 50,
|
||||
}) =>
|
||||
String? accountId,
|
||||
String? mailboxPath,
|
||||
) =>
|
||||
(super.noSuchMethod(
|
||||
Invocation.method(
|
||||
#observeEmails,
|
||||
@@ -226,17 +225,15 @@ class MockEmailRepository extends _i1.Mock implements _i9.EmailRepository {
|
||||
accountId,
|
||||
mailboxPath,
|
||||
],
|
||||
{#limit: limit},
|
||||
),
|
||||
returnValue: _i4.Stream<List<_i2.Email>>.empty(),
|
||||
) as _i4.Stream<List<_i2.Email>>);
|
||||
|
||||
@override
|
||||
_i4.Stream<List<_i2.EmailThread>> observeThreads(
|
||||
String accountId,
|
||||
String mailboxPath, {
|
||||
int limit = 50,
|
||||
}) =>
|
||||
String? accountId,
|
||||
String? mailboxPath,
|
||||
) =>
|
||||
(super.noSuchMethod(
|
||||
Invocation.method(
|
||||
#observeThreads,
|
||||
@@ -244,7 +241,6 @@ class MockEmailRepository extends _i1.Mock implements _i9.EmailRepository {
|
||||
accountId,
|
||||
mailboxPath,
|
||||
],
|
||||
{#limit: limit},
|
||||
),
|
||||
returnValue: _i4.Stream<List<_i2.EmailThread>>.empty(),
|
||||
) as _i4.Stream<List<_i2.EmailThread>>);
|
||||
|
||||
@@ -14,7 +14,7 @@ void main() {
|
||||
group('Migration', () {
|
||||
test('schemaVersion matches expected value', () async {
|
||||
final db = AppDatabase(NativeDatabase.memory());
|
||||
expect(db.schemaVersion, 26);
|
||||
expect(db.schemaVersion, 25);
|
||||
await db.close();
|
||||
});
|
||||
|
||||
@@ -158,19 +158,6 @@ void main() {
|
||||
]),
|
||||
);
|
||||
|
||||
// v26: FTS5 virtual table and triggers exist.
|
||||
final allTriggers = await db
|
||||
.customSelect("SELECT name FROM sqlite_master WHERE type='trigger'")
|
||||
.get();
|
||||
final triggerNames =
|
||||
allTriggers.map((r) => r.read<String>('name')).toSet();
|
||||
expect(
|
||||
triggerNames,
|
||||
containsAll(['email_fts_ai', 'email_fts_au', 'email_fts_ad']),
|
||||
);
|
||||
// Verify FTS table was created and is queryable.
|
||||
await db.customSelect('SELECT count(*) FROM email_fts').get();
|
||||
|
||||
await db.close();
|
||||
if (dbFile.existsSync()) dbFile.deleteSync();
|
||||
});
|
||||
@@ -289,23 +276,11 @@ void main() {
|
||||
expect(indexNames, contains('mailboxes_account_id'));
|
||||
expect(indexNames, contains('threads_latest_date'));
|
||||
|
||||
// v26: FTS5 virtual table and triggers.
|
||||
final allTriggers = await db
|
||||
.customSelect("SELECT name FROM sqlite_master WHERE type='trigger'")
|
||||
.get();
|
||||
final triggerNames =
|
||||
allTriggers.map((r) => r.read<String>('name')).toSet();
|
||||
expect(
|
||||
triggerNames,
|
||||
containsAll(['email_fts_ai', 'email_fts_au', 'email_fts_ad']),
|
||||
);
|
||||
await db.customSelect('SELECT count(*) FROM email_fts').get();
|
||||
|
||||
await db.close();
|
||||
if (dbFile.existsSync()) dbFile.deleteSync();
|
||||
});
|
||||
|
||||
test('fresh install creates all tables at schemaVersion 26', () async {
|
||||
test('fresh install creates all tables at schemaVersion 25', () async {
|
||||
final db = AppDatabase(NativeDatabase.memory());
|
||||
await db.select(db.accounts).get();
|
||||
|
||||
|
||||
@@ -79,18 +79,9 @@ class _CountingEmails implements EmailRepository {
|
||||
@override
|
||||
Future<int> flushPendingChanges(String accountId, String password) async => 0;
|
||||
@override
|
||||
Stream<List<Email>> observeEmails(
|
||||
String a,
|
||||
String m, {
|
||||
int limit = 50,
|
||||
}) =>
|
||||
Stream.value([]);
|
||||
Stream<List<Email>> observeEmails(String a, String m) => Stream.value([]);
|
||||
@override
|
||||
Stream<List<EmailThread>> observeThreads(
|
||||
String a,
|
||||
String m, {
|
||||
int limit = 50,
|
||||
}) =>
|
||||
Stream<List<EmailThread>> observeThreads(String a, String m) =>
|
||||
Stream.value([]);
|
||||
@override
|
||||
Stream<List<Email>> observeEmailsInThread(String a, String m, String t) =>
|
||||
|
||||
@@ -75,10 +75,9 @@ class MockEmailRepository extends _i1.Mock implements _i3.EmailRepository {
|
||||
|
||||
@override
|
||||
_i4.Stream<List<_i2.Email>> observeEmails(
|
||||
String accountId,
|
||||
String mailboxPath, {
|
||||
int limit = 50,
|
||||
}) =>
|
||||
String? accountId,
|
||||
String? mailboxPath,
|
||||
) =>
|
||||
(super.noSuchMethod(
|
||||
Invocation.method(
|
||||
#observeEmails,
|
||||
@@ -86,17 +85,15 @@ class MockEmailRepository extends _i1.Mock implements _i3.EmailRepository {
|
||||
accountId,
|
||||
mailboxPath,
|
||||
],
|
||||
{#limit: limit},
|
||||
),
|
||||
returnValue: _i4.Stream<List<_i2.Email>>.empty(),
|
||||
) as _i4.Stream<List<_i2.Email>>);
|
||||
|
||||
@override
|
||||
_i4.Stream<List<_i2.EmailThread>> observeThreads(
|
||||
String accountId,
|
||||
String mailboxPath, {
|
||||
int limit = 50,
|
||||
}) =>
|
||||
String? accountId,
|
||||
String? mailboxPath,
|
||||
) =>
|
||||
(super.noSuchMethod(
|
||||
Invocation.method(
|
||||
#observeThreads,
|
||||
@@ -104,7 +101,6 @@ class MockEmailRepository extends _i1.Mock implements _i3.EmailRepository {
|
||||
accountId,
|
||||
mailboxPath,
|
||||
],
|
||||
{#limit: limit},
|
||||
),
|
||||
returnValue: _i4.Stream<List<_i2.EmailThread>>.empty(),
|
||||
) as _i4.Stream<List<_i2.EmailThread>>);
|
||||
|
||||
@@ -158,19 +158,14 @@ class FakeEmailRepository implements EmailRepository {
|
||||
_emailBody = emailBody ?? const EmailBody(emailId: '', attachments: []);
|
||||
|
||||
@override
|
||||
Stream<List<Email>> observeEmails(
|
||||
String accountId,
|
||||
String mailboxPath, {
|
||||
int limit = 50,
|
||||
}) =>
|
||||
Stream<List<Email>> observeEmails(String accountId, String mailboxPath) =>
|
||||
Stream.value(List.of(_emails));
|
||||
|
||||
@override
|
||||
Stream<List<EmailThread>> observeThreads(
|
||||
String accountId,
|
||||
String mailboxPath, {
|
||||
int limit = 50,
|
||||
}) =>
|
||||
String mailboxPath,
|
||||
) =>
|
||||
observeEmails(accountId, mailboxPath).map((emails) {
|
||||
return emails.map((e) {
|
||||
return EmailThread(
|
||||
|
||||
Reference in New Issue
Block a user