Merge branch 'cu-reviewfix' into catch-up-2026-11

This commit is contained in:
Linus Rath
2026-10-31 18:54:03 +01:00
27 changed files with 840 additions and 92 deletions
+4
View File
@@ -707,8 +707,11 @@
"failed": "Could not decrypt: {reason}",
"invalid": "Invalid signature ({kind})",
"invalid_reason": "Invalid signature ({kind}): {reason}",
"key_offered": "The message contains a public key for {email}. Fingerprint: {fingerprint}",
"locked": "Encrypted ({kind}). Unlock your key to read it.",
"no_key": "Encrypted ({kind}) to a key that is not on this device.",
"save_key": "Save key for {email}",
"save_key_failed": "Could not save the key",
"self_signed": "The certificate is self-signed, not issued by a known authority.",
"signed": "Valid signature ({kind})",
"signed_by": "Valid signature ({kind}) from {signer}",
@@ -1004,6 +1007,7 @@
"section_personal": "My templates"
},
"outbox": {
"already_sent": "This message was already sent",
"discard": "Discard",
"discard_body": "It will not be sent, and its text is lost.",
"discard_title": "Discard this message?",
+23
View File
@@ -21,6 +21,8 @@ enum JmapError: Error {
case needsRefresh
case http(Int)
case badResponse
/// A /set call listed the record under notUpdated (or not under updated).
case notUpdated(String)
}
/// Credentials as the app stores them (src/api/jmap-client.ts StoredCredentials).
@@ -166,9 +168,30 @@ enum Jmap {
["PushSubscription/get", ["ids": [id], "properties": pushSubscriptionProperties], "1"],
]
let responses = try await call(apiUrl: apiUrl, authorization: authorization, using: ["urn:ietf:params:jmap:core"], methodCalls: methodCalls)
if let refused = notUpdated(responses, method: "PushSubscription/set", callId: "0", id: id) {
throw JmapError.notUpdated(refused)
}
return listedPushSubscription(responses, callId: "1", id: id)
}
/// Why the /set answer `callId` didn't update `id`, or nil when it did.
/// A method error or an answer without `id` under updated counts as refused.
static func notUpdated(_ responses: [[Any]], method: String, callId: String, id: String) -> String? {
for response in responses where response.count >= 3 && response[2] as? String == callId {
guard response[0] as? String == method, let args = response[1] as? [String: Any] else {
let args = response[1] as? [String: Any]
return (args?["type"] as? String) ?? "error"
}
if let refused = (args["notUpdated"] as? [String: Any])?[id] {
let error = refused as? [String: Any]
return (error?["description"] as? String) ?? (error?["type"] as? String) ?? "notUpdated"
}
if let updated = args["updated"] as? [String: Any], updated.keys.contains(id) { return nil }
return "not in updated"
}
return "no answer"
}
static func fetch(apiUrl: URL, accountId: String, emailIds: [String], authorization: String) async throws -> JmapFetchResult {
var request = URLRequest(url: apiUrl)
request.httpMethod = "POST"
+13 -4
View File
@@ -83,23 +83,32 @@ enum PushRenewal {
}
// The recorded expiry was stale: the server's copy has time left.
if let expires = current.expires, !needsExtension(expires) {
record(key: key, account: account, expires: expires)
record(key: key, subscriptionId: subscriptionId, expires: expires)
return false
}
let wanted = formatDate(Date().addingTimeInterval(wantedSeconds))
let extended = try await Jmap.extendPushSubscription(
apiUrl: apiUrl, id: subscriptionId, expires: wanted, authorization: authorization)
if let expires = extended?.expires { record(key: key, account: account, expires: expires) }
if let expires = extended?.expires { record(key: key, subscriptionId: subscriptionId, expires: expires) }
pushLog.info("extended subscription \(subscriptionId, privacy: .public)")
return true
} catch JmapError.notUpdated(let reason) {
// The server kept the old expiry: not extended, nothing to record.
pushLog.error("server did not extend subscription \(subscriptionId, privacy: .public): \(reason, privacy: .public)")
return false
} catch {
pushLog.error("renewal failed: \(String(describing: error), privacy: .public)")
return false
}
}
private static func record(key: String, account: [String: Any], expires: String) {
var json = account
/// Writes the expiry into the account's entry as it is now, not as it was
/// when the renewal started: the app may have replaced the subscription (or
/// the credentials key) meanwhile. Only the expiry changes, and only while
/// the entry still names the subscription that was extended.
private static func record(key: String, subscriptionId: String, expires: String) {
guard var json = SharedKeychain.readJSON(key),
json["subscriptionId"] as? String == subscriptionId else { return }
json["subscriptionExpires"] = expires
SharedKeychain.update(key, json: json)
}
@@ -40,6 +40,7 @@ import {
listScheduledEmails, RescheduleLostError, RescheduleRestoredError, rescheduleScheduledSend, sendEmail,
} from '../email';
import { RecipientsRefusedError, jmapErrorType } from '../jmap-result';
import { verdictFromCopies } from '../../lib/send-queue';
import { pruneEmailSubmissions, resetSubmissionPrune } from '../submissions';
import {
createFakeSubmissionServer, RATE_LIMITED, RELAY_DENIED, type StalwartVersion,
@@ -87,6 +88,33 @@ afterEach(() => {
expect(fake.server?.violations ?? []).toEqual([]);
});
describe('a send from an account without a Drafts folder', () => {
// The copy is made in Sent right away. If the submission is refused and
// the answer is lost, that copy must not look sent to the offline queue,
// which takes a copy without $draft as proof the message went out.
it('leaves a copy a refused, unanswered submission made marked as a draft', async () => {
const server = setup('1.0', { forbiddenAccounts: ['shared-1'] });
server.afterEachRequest(() => { throw new TypeError('Network request failed'); });
await expect(sendEmail({ ...OUTGOING, messageId: 'm1@example.com' }, 'id-1', 'sent', undefined, { accountId: 'shared-1' }))
.rejects.toThrow('Network request failed');
const copies = [...server.emails.values()];
expect(copies).toEqual([expect.objectContaining({ mailboxIds: { sent: true }, keywords: { $seen: true, $draft: true } })]);
expect(verdictFromCopies(copies)).toEqual({ kind: 'resend', staleIds: [copies[0].id] });
});
it('takes $draft off the copy once the submission went through', async () => {
const server = setup('1.0');
await sendEmail(OUTGOING, 'id-1', 'sent', undefined, {});
const copies = [...server.emails.values()];
expect(copies).toEqual([expect.objectContaining({ mailboxIds: { sent: true }, keywords: { $seen: true } })]);
expect(verdictFromCopies(copies)).toEqual({ kind: 'sent', copyId: copies[0].id });
});
});
describe('a send whose recipients the server refuses', () => {
it.each(['1.0', '0.16.23', '0.16.10', '0.16.6'] as StalwartVersion[])(
'Stalwart %s: fails when every recipient was refused, and keeps the previous draft',
@@ -101,6 +101,18 @@ live('live Stalwart: offline send queue', () => {
expect(copies.map((c) => c.id)).toEqual([first.emailId]);
}, 60_000);
it('takes $draft off a copy sent without a Drafts folder only once it went out', async () => {
const outgoing = message('no-drafts');
// Created in Sent with $draft; onSuccessUpdateEmail takes it off.
const result = await sendEmail(outgoing, identity.id, sent, undefined, {});
const copies = await findCopiesByMessageId(outgoing.messageId, lookup());
expect(copies.map((c) => c.id)).toEqual([result.emailId]);
expect(copies[0].keywords?.$draft).toBeFalsy();
expect(copies[0].keywords?.$seen).toBe(true);
const outcome = await runQueuedSend(newQueuedSend('q3', { ...payloadFor(outgoing), draftsMailboxId: undefined }, Date.now(), true), boxes);
expect(outcome.kind).toBe('alreadySent');
}, 60_000);
it('sends a marked message whose only copy is its draft, and removes the draft', async () => {
const outgoing = message('draft-only');
const draft = await saveDraftVersion(outgoing, drafts);
+26 -1
View File
@@ -18,7 +18,7 @@ vi.mock('../../lib/compose-send-account', () => ({
}));
import { jmapClient } from '../jmap-client';
import { findCopiesByMessageId, runQueuedSend } from '../queued-send';
import { findCopiesByMessageId, runQueuedSend, wasQueuedSendSent } from '../queued-send';
import { newQueuedSend, type QueuedSendPayload } from '../../lib/send-queue';
const mockRequest = jmapClient.request as ReturnType<typeof vi.fn>;
@@ -85,6 +85,31 @@ describe('runQueuedSend', () => {
});
});
describe('wasQueuedSendSent', () => {
it('finds a copy an earlier attempt sent, and removes the old draft', async () => {
mockRequest.mockResolvedValueOnce(found([
{ id: 'draft-1', keywords: { $draft: true }, messageId: ['m1@x.de'] },
{ id: 'sent-1', keywords: { $seen: true }, messageId: ['m1@x.de'] },
]));
expect(await wasQueuedSendSent(newQueuedSend('q', PAYLOAD, 0, true), [])).toBe(true);
expect(destroyEmails).toHaveBeenCalledWith(['draft-1'], 'acc-1');
});
it('says not sent when only drafts are there, and removes nothing', async () => {
mockRequest.mockResolvedValueOnce(found([
{ id: 'draft-1', keywords: { $draft: true }, messageId: ['m1@x.de'] },
{ id: 'left-1', keywords: { $draft: true, $seen: true }, messageId: ['m1@x.de'] },
]));
expect(await wasQueuedSendSent(newQueuedSend('q', PAYLOAD, 0, true), [])).toBe(false);
expect(destroyEmails).not.toHaveBeenCalled();
});
it('does not look for a message no attempt reached the server with', async () => {
expect(await wasQueuedSendSent(newQueuedSend('q', PAYLOAD, 0, false), [])).toBe(false);
expect(mockRequest).not.toHaveBeenCalled();
});
});
describe('findCopiesByMessageId', () => {
it('reads the recent messages of Sent and Drafts and keeps exact matches only', async () => {
mockRequest.mockResolvedValueOnce(found([
+10 -9
View File
@@ -1781,8 +1781,12 @@ export async function sendEmail(
emailCreate.mailboxIds = { [opts!.draftsMailboxId!]: true };
emailCreate.keywords = { $seen: true, $draft: true };
} else {
// No Drafts folder: created in Sent, but still with `$draft` until the
// submission succeeds, so a copy whose submission was refused (with the
// answer lost) never passes for a sent message. The send queue takes a
// copy without `$draft` as proof the message went out (verdictFromCopies).
emailCreate.mailboxIds = { [sentMailboxId]: true };
emailCreate.keywords = { $seen: true };
emailCreate.keywords = { $seen: true, $draft: true };
}
const createCall: [string, Record<string, unknown>, string] = raw
? ['Email/import', { accountId, emails: { draft: emailCreate } }, '0']
@@ -1818,14 +1822,11 @@ export async function sendEmail(
accountId,
create: { 'sub-1': submissionCreate },
};
if (viaDrafts) {
submissionArgs.onSuccessUpdateEmail = {
'#sub-1': {
mailboxIds: { [sentMailboxId]: true },
[keywordPointer('$draft')]: null,
},
};
}
submissionArgs.onSuccessUpdateEmail = {
'#sub-1': viaDrafts
? { mailboxIds: { [sentMailboxId]: true }, [keywordPointer('$draft')]: null }
: { [keywordPointer('$draft')]: null },
};
let res = await jmapClient.request(
[
+54 -24
View File
@@ -8,7 +8,7 @@ import type { Mailbox } from './types';
import { sendTargetFor } from '../lib/compose-send-account';
import { supportsSubmissionExtension } from '../lib/submission-options';
import { stripMessageIdBrackets } from '../lib/email-threading';
import { holdForScheduled, verdictFromCopies, type FoundCopy, type QueuedSend } from '../lib/send-queue';
import { holdForScheduled, verdictFromCopies, type CopyVerdict, type FoundCopy, type QueuedSend } from '../lib/send-queue';
/** How far before the queueing a copy can be dated (the server's clock may differ). */
const CLOCK_SKEW_MS = 10 * 60_000;
@@ -68,34 +68,64 @@ export type QueuedSendOutcome =
* went out; copies a refused attempt left in Drafts are removed before the
* new attempt.
*/
export async function runQueuedSend(send: QueuedSend, mailboxes: Mailbox[], now = Date.now()): Promise<QueuedSendOutcome> {
/** The account and folders a queued message goes through, as the composer works them out. */
async function queuedSendTarget(send: QueuedSend, mailboxes: Mailbox[]) {
const p = send.payload;
const target = await sendTargetFor(p.identity, p.fromEmail, mailboxes);
const accountId = target.accountId ?? jmapClient.accountId;
const draftAccountId = p.draftAccountId ?? jmapClient.accountId;
return {
target,
accountId: target.accountId ?? jmapClient.accountId,
draftAccountId: p.draftAccountId ?? jmapClient.accountId,
sentMailboxId: target.sentMailboxId ?? p.sentMailboxId,
draftsMailboxId: target.accountId ? target.draftsMailboxId : p.draftsMailboxId,
};
}
const sentMailboxId = target.sentMailboxId ?? p.sentMailboxId;
const draftsMailboxId = target.accountId ? target.draftsMailboxId : p.draftsMailboxId;
type Resolved = Awaited<ReturnType<typeof queuedSendTarget>>;
/**
* Whether an earlier, unanswered attempt at `send` went out. When it did, the
* old draft that send would have removed is removed now.
*/
async function earlierAttempt(send: QueuedSend, r: Resolved): Promise<CopyVerdict> {
const p = send.payload;
const copies = await findCopiesByMessageId(p.outgoing.messageId!, {
accountId: r.target.accountId,
mailboxIds: [r.sentMailboxId, r.draftsMailboxId ?? ''],
since: send.createdAt,
});
// The autosaved draft lives in this account only when the draft did.
const draftHere = p.draftId && r.draftAccountId === r.accountId ? p.draftId : undefined;
const verdict = verdictFromCopies(copies, draftHere);
if (verdict.kind === 'sent' && p.draftId && p.draftId !== verdict.copyId) {
// The send went out but its answer was lost, so the old draft it would
// have removed is still there.
await destroyEmails([p.draftId], r.draftAccountId).catch((err) => {
console.warn('[send-queue] could not remove the draft of a sent message', err);
});
}
return verdict;
}
/**
* Before a queued message is taken back (moved to Drafts): true when an
* earlier attempt that may have reached the server did send it. A message no
* attempt reached the server with is never looked up.
*/
export async function wasQueuedSendSent(send: QueuedSend, mailboxes: Mailbox[]): Promise<boolean> {
if (!send.maybeSent || !send.payload.outgoing.messageId) return false;
const verdict = await earlierAttempt(send, await queuedSendTarget(send, mailboxes));
return verdict.kind === 'sent';
}
export async function runQueuedSend(send: QueuedSend, mailboxes: Mailbox[], now = Date.now()): Promise<QueuedSendOutcome> {
const p = send.payload;
const resolved = await queuedSendTarget(send, mailboxes);
const { target, accountId, sentMailboxId, draftsMailboxId } = resolved;
if (send.maybeSent && p.outgoing.messageId) {
const copies = await findCopiesByMessageId(p.outgoing.messageId, {
accountId: target.accountId,
mailboxIds: [sentMailboxId, draftsMailboxId ?? ''],
since: send.createdAt,
});
// The autosaved draft lives in this account only when the draft did.
const draftHere = p.draftId && draftAccountId === accountId ? p.draftId : undefined;
const verdict = verdictFromCopies(copies, draftHere);
if (verdict.kind === 'sent') {
// The send went out but its answer was lost, so the old draft it would
// have removed is still there.
if (p.draftId && p.draftId !== verdict.copyId) {
await destroyEmails([p.draftId], draftAccountId).catch((err) => {
console.warn('[send-queue] could not remove the draft of a sent message', err);
});
}
return { kind: 'alreadySent', accountId: target.accountId };
}
const verdict = await earlierAttempt(send, resolved);
if (verdict.kind === 'sent') return { kind: 'alreadySent', accountId: target.accountId };
if (verdict.staleIds.length > 0) {
await destroyEmails(verdict.staleIds, accountId);
}
+1 -1
View File
@@ -130,7 +130,7 @@ export function MessageContent({
/>
)}
{(secure.loading || secure.result) && <SecureMessageBanner status={secure.result?.status ?? null} loading={secure.loading} />}
{(secure.loading || secure.result) && <SecureMessageBanner status={secure.result?.status ?? null} loading={secure.loading} offeredKey={secure.result?.offeredKey} />}
<CalendarInvitationBanner email={email} jmapAccountId={jmapAccountId} />
<StructuredMailBanner email={email} jmapAccountId={jmapAccountId} />
+30 -5
View File
@@ -7,16 +7,22 @@ import { spacing, radius, typography, type ThemePalette } from '../../theme/toke
import { useColors } from '../../theme/colors';
import { useLocaleStore } from '../../stores/locale-store';
import { useCryptoKeysStore, type CryptoKeyRecord } from '../../stores/crypto-keys-store';
import type { SecureStatus } from '../../lib/crypto/secure-message';
import { secureBannerRows, type BannerTone as Tone } from '../../lib/crypto/banner-rows';
import type { OfferedPgpKey, SecureStatus } from '../../lib/crypto/secure-message';
import { offeredKeyRow, secureBannerRows, type BannerTone as Tone } from '../../lib/crypto/banner-rows';
import { savePgpKeyFor } from '../../lib/crypto/keyring';
import { toast } from '../../stores/toast-store';
/** Status banner above a PGP / S/MIME message, with Unlock for a locked key. */
export function SecureMessageBanner({ status, loading }: { status: SecureStatus | null; loading?: boolean }) {
/**
* Status banner above a PGP / S/MIME message, with Unlock for a locked key
* and Save for a public key the message carries for its sender.
*/
export function SecureMessageBanner({ status, loading, offeredKey }: { status: SecureStatus | null; loading?: boolean; offeredKey?: OfferedPgpKey }) {
const c = useColors();
const styles = React.useMemo(() => makeStyles(c), [c]);
const t = useLocaleStore((s) => s.t);
const keys = useCryptoKeysStore((s) => s.keys);
const [unlockRecord, setUnlockRecord] = React.useState<CryptoKeyRecord | null>(null);
const [saving, setSaving] = React.useState(false);
if (loading) {
return (
@@ -28,11 +34,23 @@ export function SecureMessageBanner({ status, loading }: { status: SecureStatus
}
if (!status) return null;
const rows = secureBannerRows(status, t);
if (rows.length === 0) return null;
const color = (tone: Tone) => (tone === 'ok' ? c.success : tone === 'warn' ? c.warning : tone === 'error' ? c.error : c.textMuted);
const lockedRecord = status.lockedKeyIds?.length
? keys.find((k) => k.kind === status.kind && status.lockedKeyIds!.includes(k.id)) ?? null
: null;
const offer = offeredKeyRow(offeredKey, keys, t);
if (rows.length === 0 && !offer) return null;
const saveOffered = async () => {
if (!offeredKey) return;
setSaving(true);
try {
await savePgpKeyFor(offeredKey.armored, offeredKey.email);
} catch (err) {
toast.error(t('encryption.banner.save_key_failed', 'Could not save the key'), { message: err instanceof Error ? err.message : String(err) });
} finally {
setSaving(false);
}
};
return (
<View style={styles.wrap} accessibilityRole="summary">
@@ -48,6 +66,13 @@ export function SecureMessageBanner({ status, loading }: { status: SecureStatus
</View>
);
})}
{offer && (
<View style={[styles.row, { borderColor: c.border }]}>
<Shield size={16} color={c.textMuted} />
<Text style={[styles.text, { color: c.textMuted }]}>{offer.text}</Text>
<Button size="sm" variant="outline" disabled={saving} onPress={() => { void saveOffered(); }}>{offer.action}</Button>
</View>
)}
<UnlockKeyDialog
record={unlockRecord}
reason={t('encryption.unlock_read_reason', 'The message is encrypted to this key.')}
+1 -2
View File
@@ -3,7 +3,7 @@ import type { Email } from '../../api/types';
import { fetchBlobBytes } from '../../lib/email-export';
import { secureKindHint, openSecureMessage, type SecureKeyAccess, type SecureResult } from '../../lib/crypto/secure-message';
import {
unlockedPgpKeys, ownKeys, allPgpPublicKeys, isUnlocked, unlockedSmimeKey, onUnlockChange, rememberSignerCertificate, rememberSignerPgpKey,
unlockedPgpKeys, ownKeys, allPgpPublicKeys, isUnlocked, unlockedSmimeKey, onUnlockChange, rememberSignerCertificate,
unlockIfUnprotected,
} from '../../lib/crypto/keyring';
import { base64Decode } from '../../lib/crypto/mime';
@@ -17,7 +17,6 @@ export function keyringAccess(): SecureKeyAccess {
pgpOwnKeys: () => (ownKeys('pgp') as PgpKeyRecord[]).map((k) => ({ id: k.id, publicKeyArmored: k.publicKeyArmored, unlocked: isUnlocked(k) })),
pgpPublicKeys: allPgpPublicKeys,
smimeKeys: () => (ownKeys('smime') as SmimeCertRecord[]).map((k) => ({ id: k.id, certificate: base64Decode(k.certificate), pkcs8: unlockedSmimeKey(k.id)?.pkcs8 })),
rememberPgpKey: rememberSignerPgpKey,
rememberCertificate: rememberSignerCertificate,
};
}
@@ -143,6 +143,36 @@ describe('the native renewal', () => {
expect(set).toBeGreaterThan(own);
});
it('treats an update the server refused as not extended, and records nothing', () => {
const jmap = read('plugins/ios-push/Jmap.swift');
const extend = jmap.slice(jmap.indexOf('static func extendPushSubscription('));
const check = extend.indexOf('notUpdated(responses, method: "PushSubscription/set", callId: "0", id: id)');
expect(check).toBeGreaterThan(-1);
expect(extend.indexOf('throw JmapError.notUpdated(')).toBeGreaterThan(check);
expect(check).toBeLessThan(extend.indexOf('return listedPushSubscription(responses, callId: "1", id: id)'));
// The renewal stops on it before logging "extended" or recording.
const body = renewal.slice(renewal.indexOf('static func renew(key:'), renewal.indexOf('private static func record('));
const refused = body.match(/catch JmapError\.notUpdated[\s\S]*?\}/)?.[0] ?? '';
expect(refused).toContain('return false');
expect(refused).not.toContain('record(');
expect(body.indexOf('try await Jmap.extendPushSubscription(')).toBeLessThan(body.indexOf('pushLog.info("extended subscription'));
});
it('writes back only the expiry, into the entry as it is now, while it names the same subscription', () => {
const record = renewal.slice(renewal.indexOf('private static func record('));
expect(record).toMatch(/^private static func record\(key: String, subscriptionId: String, expires: String\)/);
expect(record).toMatch(/guard var json = SharedKeychain\.readJSON\(key\),\s*json\["subscriptionId"\] as\? String == subscriptionId else \{ return \}/);
expect(record).toMatch(/json\["subscriptionExpires"\] = expires\s*SharedKeychain\.update\(key, json: json\)/);
expect(renewal).not.toMatch(/record\(key: key, account:/);
});
it('reads a refused update the way JMAP reports it', () => {
const jmap = read('plugins/ios-push/Jmap.swift');
const helper = jmap.slice(jmap.indexOf('static func notUpdated('));
expect(helper).toMatch(/args\["notUpdated"\] as\? \[String: Any\]\)\?\[id\]/);
expect(helper).toMatch(/updated\.keys\.contains\(id\) \{ return nil \}/);
});
it('runs from the notification extension after the fetch for the pushed account', () => {
const renew = nse.indexOf('await PushRenewal.renewIfDue(jmapAccountId: accountId)');
expect(renew).toBeGreaterThan(nse.indexOf('await NotificationService.enrich('));
+20 -1
View File
@@ -1,6 +1,6 @@
// What the reader's banner says for each PGP / S/MIME outcome (#58).
import { describe, expect, it } from 'vitest';
import { secureBannerRows } from '../banner-rows';
import { offeredKeyRow, secureBannerRows } from '../banner-rows';
const t = (_k: string, fallback: string, vars?: Record<string, string | number>) =>
fallback.replace(/\{(\w+)\}/g, (_, name: string) => String(vars?.[name] ?? ''));
@@ -29,3 +29,22 @@ describe('secureBannerRows', () => {
expect(secureBannerRows({ kind: 'pgp', encrypted: false, decrypted: false, signature: 'none' }, t)).toEqual([]);
});
});
describe('offeredKeyRow', () => {
const offer = { armored: 'k', fingerprint: 'abcd1234abcd1234abcd1234abcd1234abcd1234', email: 'a@example.org' };
it('offers to save the key for the sender, with its fingerprint', () => {
expect(offeredKeyRow(offer, [], t)).toEqual({
key: 'offer',
tone: 'muted',
text: 'The message contains a public key for a@example.org. Fingerprint: ABCD 1234 ABCD 1234 ABCD 1234 ABCD 1234 ABCD 1234',
action: 'Save key for a@example.org',
});
});
it('offers nothing without a key, or once that key is stored', () => {
expect(offeredKeyRow(undefined, [], t)).toBeNull();
expect(offeredKeyRow(offer, [{ kind: 'pgp', id: offer.fingerprint }], t)).toBeNull();
expect(offeredKeyRow(offer, [{ kind: 'smime', id: offer.fingerprint }], t)).not.toBeNull();
});
});
@@ -102,6 +102,18 @@ describe('prepareSecureSend', () => {
expect(new TextDecoder().decode(uploaded!)).not.toMatch(/bcc@example.org/i);
}, 60_000);
it('quotes a Reply-To name with a comma so it stays one address', async () => {
await importPgpKey(me.privateKey, me.passphrase);
let uploaded: Uint8Array | null = null;
const msg = { ...outgoing(), replyTo: [{ name: 'Support, ACME', email: 'help@acme.com' }] };
await prepareSecureSend(msg, { mode: 'pgp', sign: true, encrypt: false }, {
fetchBlob: async () => new Uint8Array([1]),
upload: async (bytes) => { uploaded = bytes; return 'x'; },
});
const text = new TextDecoder().decode(uploaded!);
expect(text).toMatch(/^Reply-To: "Support, ACME" <help@acme.com>$/m);
}, 60_000);
it('stops when a recipient has no key, or when the signing key is locked', async () => {
await importPgpKey(me.privateKey, me.passphrase);
const deps = { fetchBlob: async () => new Uint8Array(), upload: async () => 'x' };
@@ -0,0 +1,137 @@
// Keys that arrive in a message (#58): any sender can put a public key in a
// message, and the app used to store every one it found, for whatever address
// it named, whatever the signature said. Stored keys verify signatures and
// are encrypted to, so that let a sender plant a key for someone else. Now a
// key is only offered (when it names the From address) and saved by the
// user; an S/MIME certificate is kept only when it names the From address.
import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
import * as openpgp from 'openpgp';
import fs from 'node:fs';
import path from 'node:path';
import { buildSecureMessage } from '../secure-send';
import { openSecureMessage } from '../secure-message';
import { importPgpKey, lockAll, savePgpKeyFor } from '../keyring';
import { useCryptoKeysStore } from '../../../stores/crypto-keys-store';
import { keyringAccess } from '../../../components/email/use-secure-message';
import type { MimeMessageInput } from '../mime';
import { makePgpKey, makeSmimeCert, useNodeCrypto, type TestCert } from './helpers';
const secure = vi.hoisted(() => new Map<string, string>());
vi.mock('expo-secure-store', () => ({
WHEN_UNLOCKED_THIS_DEVICE_ONLY: 6,
setItemAsync: vi.fn(async (k: string, v: string) => { secure.set(k, v); }),
getItemAsync: vi.fn(async (k: string) => secure.get(k) ?? null),
deleteItemAsync: vi.fn(async (k: string) => { secure.delete(k); }),
}));
const encode = (s: string) => new TextEncoder().encode(s);
const stored = () => useCryptoKeysStore.getState().keys;
let alice: Awaited<ReturnType<typeof makePgpKey>>;
let mallory: Awaited<ReturnType<typeof makePgpKey>>;
let bob: Awaited<ReturnType<typeof makePgpKey>>;
beforeAll(async () => {
useNodeCrypto();
[alice, mallory, bob] = await Promise.all([
makePgpKey('alice@example.org'), makePgpKey('mallory@example.org'), makePgpKey('bob@example.org'),
]);
}, 60_000);
beforeEach(() => {
secure.clear();
lockAll();
useCryptoKeysStore.setState({ keys: [] });
});
/** An inline cleartext-signed message from `from` whose text carries `key`. */
async function inlineSigned(from: string, signer: openpgp.PrivateKey, key: string, tamper = false): Promise<Uint8Array> {
const signed = await openpgp.sign({
message: await openpgp.createCleartextMessage({ text: `Here is my key.\n\n${key}` }),
signingKeys: signer,
}) as string;
const body = tamper ? signed.replace('Here is my key.', 'Here is my kex.') : signed;
return encode(`From: ${from}\r\nContent-Type: text/plain\r\n\r\n${body.replace(/\r?\n/g, '\r\n')}`);
}
describe('a PGP key in a message', () => {
it('is offered, not stored, when it names the From address and the signer is not known yet', async () => {
const res = await openSecureMessage(await inlineSigned(alice.email, alice.unlocked, alice.publicKey), alice.email, keyringAccess());
expect(res?.status.signature).toBe('unknown');
const fingerprint = (await openpgp.readKey({ armoredKey: alice.publicKey })).getFingerprint();
expect(res?.offeredKey).toMatchObject({ email: alice.email, fingerprint });
expect(stored()).toEqual([]);
// Stored only once the user saves it.
await savePgpKeyFor(res!.offeredKey!.armored, res!.offeredKey!.email);
expect(stored()).toEqual([expect.objectContaining({ kind: 'pgp', id: fingerprint, emails: [alice.email], source: 'message' })]);
}, 60_000);
it('is neither stored nor offered when none of its user ids is the From address', async () => {
const res = await openSecureMessage(await inlineSigned(alice.email, alice.unlocked, mallory.publicKey), alice.email, keyringAccess());
expect(res?.offeredKey).toBeUndefined();
expect(stored()).toEqual([]);
await expect(savePgpKeyFor(mallory.publicKey, alice.email)).rejects.toThrow(/not for alice@example.org/);
expect(stored()).toEqual([]);
}, 60_000);
it('is neither stored nor offered when the signature fails', async () => {
await importPgpKey(alice.publicKey);
const res = await openSecureMessage(await inlineSigned(alice.email, alice.unlocked, alice.publicKey, true), alice.email, keyringAccess());
expect(res?.status.signature).toBe('invalid');
expect(res?.offeredKey).toBeUndefined();
expect(stored()).toHaveLength(1);
}, 60_000);
it('is not stored from a decrypted message either', async () => {
await importPgpKey(bob.privateKey, bob.passphrase);
const input: MimeMessageInput = {
from: { email: alice.email },
to: [{ email: bob.email }],
subject: 'key',
textBody: `My new key:\n\n${mallory.publicKey}`,
};
const raw = await buildSecureMessage(input, { mode: 'pgp', sign: false, encrypt: true }, { pgp: { encryptionKeys: [bob.publicKey] } });
const access = { ...keyringAccess(), pgpDecryptionKeys: () => [bob.unlocked], pgpOwnKeys: () => [{ id: 'bob', publicKeyArmored: bob.publicKey, unlocked: true }] };
const res = await openSecureMessage(raw, alice.email, access);
expect(res?.status.decrypted).toBe(true);
expect(res?.body?.text).toContain('BEGIN PGP PUBLIC KEY BLOCK');
expect(res?.offeredKey).toBeUndefined();
expect(stored().map((k) => k.emails)).toEqual([[bob.email]]);
}, 60_000);
});
describe('an S/MIME signer certificate', () => {
let signer: TestCert;
beforeAll(async () => {
signer = await makeSmimeCert('alice@example.org');
}, 60_000);
const signed = () => buildSecureMessage(
{ from: { email: signer.email }, to: [{ email: 'bob@example.org' }], subject: 's', textBody: 'signed' },
{ mode: 'smime', sign: true, encrypt: false },
{ smime: { signer: { pkcs8: signer.pkcs8, certificate: signer.certificate, chain: [], algorithm: 'RSA-2048' } } },
);
it('is not kept when it does not name the From address, even with a valid signature', async () => {
const res = await openSecureMessage(await signed(), 'mallory@example.org', keyringAccess());
expect(res?.status.signature).toBe('valid');
expect(stored()).toEqual([]);
}, 60_000);
it('is kept when it names the From address', async () => {
await openSecureMessage(await signed(), signer.email, keyringAccess());
await vi.waitFor(() => expect(stored()).toEqual([expect.objectContaining({ kind: 'smime', emails: [signer.email], source: 'message' })]));
}, 60_000);
});
describe('the reader', () => {
it('hands the offered key to the banner, which saves it only on the button', () => {
const root = path.resolve(__dirname, '../../../..');
const content = fs.readFileSync(path.join(root, 'src/components/email/MessageContent.tsx'), 'utf8');
expect(content).toContain('offeredKey={secure.result?.offeredKey}');
const banner = fs.readFileSync(path.join(root, 'src/components/email/SecureMessageBanner.tsx'), 'utf8');
expect(banner).toMatch(/onPress=\{\(\) => \{ void saveOffered\(\); \}\}>\{offer\.action\}/);
expect(banner.match(/savePgpKeyFor\(/g)).toHaveLength(1);
});
});
+26 -1
View File
@@ -1,5 +1,6 @@
// Banner rows for a PGP / S/MIME status (#58).
import type { SecureStatus } from './secure-message';
import type { OfferedPgpKey, SecureStatus } from './secure-message';
import { formatFingerprint } from './key-labels';
export type BannerTone = 'ok' | 'warn' | 'error' | 'muted';
type Translate = (key: string, fallback: string, vars?: Record<string, string | number>) => string;
@@ -49,3 +50,27 @@ export function secureBannerRows(status: SecureStatus, t: Translate): BannerRow[
return rows;
}
/**
* The row that offers to save the public key a message carries for its
* From address, with its fingerprint to compare, and the label of the save
* button. Null without an offer, or once a key with that fingerprint is
* stored.
*/
export function offeredKeyRow(
offer: OfferedPgpKey | undefined,
stored: ReadonlyArray<{ kind: string; id: string }>,
t: Translate,
): (BannerRow & { action: string }) | null {
if (!offer) return null;
if (stored.some((k) => k.kind === 'pgp' && k.id === offer.fingerprint)) return null;
return {
key: 'offer',
tone: 'muted',
text: t('encryption.banner.key_offered', 'The message contains a public key for {email}. Fingerprint: {fingerprint}', {
email: offer.email,
fingerprint: formatFingerprint(offer.fingerprint),
}),
action: t('encryption.banner.save_key', 'Save key for {email}', { email: offer.email }),
};
}
+2 -2
View File
@@ -8,7 +8,7 @@ import type { OutgoingEmail } from '../../api/email';
import type { ContactCard } from '../../api/types';
import { useCryptoKeysStore, keysForEmail, isRecordCurrent, type CryptoKeyRecord, type PgpKeyRecord, type SmimeCertRecord } from '../../stores/crypto-keys-store';
import { messagePriorityHeaders } from '../message-priority';
import { base64Decode, formatHeader, encodeHeaderWords, type MimeAttachment, type MimeMessageInput } from './mime';
import { base64Decode, formatHeader, formatAddress, type MimeAttachment, type MimeMessageInput } from './mime';
import { buildSecureMessage, type SecureMode } from './secure-send';
import { ownKeyFor, unlockedPgpKey, unlockedSmimeKey } from './keyring';
@@ -100,7 +100,7 @@ export function secureReadiness(choice: SecureChoice, fromEmail: string, recipie
function extraHeaders(outgoing: OutgoingEmail): string[] {
const lines: string[] = [];
if (outgoing.replyTo?.length) {
lines.push(formatHeader('Reply-To', outgoing.replyTo.map((a) => (a.name ? `${encodeHeaderWords(a.name)} <${a.email}>` : a.email)).join(', ')));
lines.push(formatHeader('Reply-To', outgoing.replyTo.map(formatAddress).join(', ')));
}
for (const [prop, value] of Object.entries(messagePriorityHeaders(outgoing.priority))) {
const name = /^header:([^:]+):/.exec(prop)?.[1];
+26 -11
View File
@@ -219,23 +219,38 @@ export function allPgpPublicKeys(): string[] {
return records().filter((k): k is PgpKeyRecord => k.kind === 'pgp').map((k) => k.publicKeyArmored);
}
/** Remember the key of someone who signed a message, unless it is known already. */
export async function rememberSignerPgpKey(armored: string): Promise<void> {
try {
const info = await inspectKey(armored);
if (info.isPrivate) return;
if (records().some((k) => k.kind === 'pgp' && k.id === info.fingerprint.toLowerCase())) return;
upsert(pgpRecord(info, false, 'message'));
} catch {
// a broken embedded key: ignore
/**
* Save a public key that came in a message from `email`, when the user asks
* to (the reader's banner offers it). Refused unless one of the key's user
* ids is that address: a sender must not plant a key for someone else.
*/
export async function savePgpKeyFor(armored: string, email: string): Promise<PgpKeyRecord> {
const info = await inspectKey(armored);
if (info.isPrivate) throw new Error('Only a public key can be saved from a message');
const address = email.trim().toLowerCase();
if (!address || !info.userIDs.some((u) => u.email.trim().toLowerCase() === address)) {
throw new Error('The key is not for ' + email);
}
const known = records().find((k): k is PgpKeyRecord => k.kind === 'pgp' && k.id === info.fingerprint.toLowerCase());
if (known) return known;
const record = pgpRecord(info, false, 'message');
upsert(record);
// Open messages verify again with the key.
notify();
return record;
}
export async function rememberSignerCertificate(der: Uint8Array): Promise<void> {
/**
* Keep the certificate of a valid S/MIME signature on a message from
* `fromEmail`, unless it is known already. Only one that names that address
* (subject email or SAN rfc822Name) is kept.
*/
export async function rememberSignerCertificate(der: Uint8Array, fromEmail: string): Promise<void> {
try {
const { describeCertificate } = await import('./smime/certificate');
const info = await describeCertificate(der);
if (!info.emailAddresses.length) return;
const from = fromEmail.trim().toLowerCase();
if (!from || !info.emailAddresses.some((e) => e.toLowerCase() === from)) return;
if (records().some((k) => k.kind === 'smime' && k.id === info.fingerprint)) return;
upsert(smimeRecord(info, der, [], false, 'message'));
} catch {
+2 -1
View File
@@ -90,7 +90,8 @@ function unfold(value: string | undefined): string {
return String(value ?? '').replace(/[\r\n\t]+/g, ' ').replace(/ {2,}/g, ' ').trim();
}
function formatAddress(addr: MimeAddress): string {
/** One address for a header: the name quoted (or RFC 2047-encoded) so commas in it stay inside. */
export function formatAddress(addr: MimeAddress): string {
if (addr.name) {
const name = unfold(addr.name);
// Raw 8-bit in a header is rejected by some MTAs; encode it.
+41 -12
View File
@@ -9,7 +9,7 @@ import type { BodyPart, Email } from '../../api/types';
import { binaryToBytes, bytesToBinary, decodeBody, findNode, parseEntity, type MimeNode } from './mime';
import {
detectPgp, decryptMessage, verifyDetached, verifyCleartext, encryptionKeyIds, keyHasId, canonicalSignedBytes,
extractPublicKeyBlock, PgpArgon2UnsupportedError, type PgpSignatureSummary,
extractPublicKeyBlock, inspectKey, PgpArgon2UnsupportedError, type PgpSignatureSummary,
} from './pgp';
import type { SmimeKeyMaterial, SmimeSignatureStatus } from './smime/cms';
@@ -43,11 +43,24 @@ export interface SecureAttachment {
bytes: Uint8Array;
}
/**
* A public key in the message for its From address. Never saved on its own:
* any sender can put a key in a message, so the banner offers it and only
* the user saves it (keyring savePgpKeyFor).
*/
export interface OfferedPgpKey {
armored: string;
fingerprint: string;
/** The From address, which one of the key's user ids names. */
email: string;
}
export interface SecureResult {
status: SecureStatus;
/** The decrypted/verified body, or null when there is none to show (locked, failed). */
body: { html: string | null; text: string | null } | null;
attachments: SecureAttachment[];
offeredKey?: OfferedPgpKey;
}
/** What the reader needs from the key store; injected so tests need no SecureStore. */
@@ -60,8 +73,8 @@ export interface SecureKeyAccess {
pgpPublicKeys(): string[];
/** Own S/MIME keys; `pkcs8` set when unlocked. */
smimeKeys(): SmimeKeyMaterial[];
rememberPgpKey?(armored: string): Promise<void>;
rememberCertificate?(der: Uint8Array): Promise<void>;
/** Keep the certificate of a valid signature, if it names `fromEmail`. */
rememberCertificate?(der: Uint8Array, fromEmail: string): Promise<void>;
}
type PartLike = Pick<BodyPart, 'type'> & { subParts?: PartLike[] };
@@ -164,8 +177,7 @@ async function openPgp(raw: Uint8Array, fromEmail: string | undefined, keys: Sec
}
applyPgpSignature(status, sig, fromEmail);
const rendered = await renderPayload(payload);
await maybeRememberEmbeddedKey(rendered.body?.text, keys);
return { status, ...rendered };
return withOfferedKey({ status, ...rendered }, rendered.body?.text, fromEmail);
}
if (info.kind === 'pgp-mime-signed') {
applyPgpSignature(status, await verifySignedPart(info.signedBytes, info.signature, verificationKeys), fromEmail);
@@ -174,8 +186,7 @@ async function openPgp(raw: Uint8Array, fromEmail: string | undefined, keys: Sec
// inline-signed
const v = await verifyCleartext({ cleartext: info.cleartext, verificationKeysArmored: verificationKeys });
applyPgpSignature(status, v.signature, fromEmail);
await maybeRememberEmbeddedKey(v.text, keys);
return { status, body: { html: null, text: v.text }, attachments: [] };
return withOfferedKey({ status, body: { html: null, text: v.text }, attachments: [] }, v.text, fromEmail);
} catch (err) {
status.error = err instanceof PgpArgon2UnsupportedError ? err.message : err instanceof Error ? err.message : String(err);
if (status.encrypted) status.decrypted = false;
@@ -194,9 +205,27 @@ async function verifySignedPart(signed: Uint8Array, signature: string, verificat
return retry.status === 'valid' ? retry : first;
}
async function maybeRememberEmbeddedKey(text: string | null | undefined, keys: SecureKeyAccess): Promise<void> {
/**
* Offers a public key the text carries, only when one of its user ids is the
* From address and the signature (if any) did not fail. Nothing is stored
* here: the webmail's bulwark-gpg plugin imports keys only from its
* settings, and a key that arrived in a message would otherwise be used to
* verify and encrypt for whatever address it claims.
*/
async function withOfferedKey(result: SecureResult, text: string | null | undefined, fromEmail: string | undefined): Promise<SecureResult> {
const from = fromEmail?.trim().toLowerCase();
if (!from || result.status.signature === 'invalid') return result;
const block = extractPublicKeyBlock(text ?? '');
if (block && keys.rememberPgpKey) await keys.rememberPgpKey(block);
if (!block) return result;
try {
const info = await inspectKey(block);
if (info.isPrivate) return result;
if (!info.userIDs.some((u) => u.email.trim().toLowerCase() === from)) return result;
return { ...result, offeredKey: { armored: info.publicKeyArmored, fingerprint: info.fingerprint.toLowerCase(), email: from } };
} catch {
// a broken embedded key: nothing to offer
return result;
}
}
function applySmimeSignature(status: SecureStatus, sig: SmimeSignatureStatus): void {
@@ -228,7 +257,7 @@ async function openSmime(raw: Uint8Array, fromEmail: string | undefined, keys: S
const entity = binaryToBytes(signed.raw);
const sig = await cms.smimeVerifyDetached(decodeBody(sigPart), entity, fromEmail);
applySmimeSignature(status, sig);
if (sig.signatureValid && sig.signerCertificate) await keys.rememberCertificate?.(sig.signerCertificate);
if (sig.signatureValid && sig.signerCertificate && fromEmail) await keys.rememberCertificate?.(sig.signerCertificate, fromEmail);
return { status, ...(await renderPayload(entity)) };
}
@@ -236,7 +265,7 @@ async function openSmime(raw: Uint8Array, fromEmail: string | undefined, keys: S
if (cms.isSignedData(der)) {
const v = await cms.smimeVerifyOpaque(der, fromEmail);
applySmimeSignature(status, v.status);
if (v.status.signatureValid && v.status.signerCertificate) await keys.rememberCertificate?.(v.status.signerCertificate);
if (v.status.signatureValid && v.status.signerCertificate && fromEmail) await keys.rememberCertificate?.(v.status.signerCertificate, fromEmail);
return { status, ...(await renderPayload(v.mimeBytes)) };
}
@@ -274,7 +303,7 @@ async function openSmime(raw: Uint8Array, fromEmail: string | undefined, keys: S
if (innerSigned && cms.isSignedData(innerSigned)) {
const v = await cms.smimeVerifyOpaque(innerSigned, fromEmail);
applySmimeSignature(status, v.status);
if (v.status.signatureValid && v.status.signerCertificate) await keys.rememberCertificate?.(v.status.signerCertificate);
if (v.status.signatureValid && v.status.signerCertificate && fromEmail) await keys.rememberCertificate?.(v.status.signerCertificate, fromEmail);
payload = v.mimeBytes;
} else if (inner.type === 'multipart/signed' && /pkcs7-signature/i.test(inner.params.protocol || '')) {
const full = parseEntity(bytesToBinary(decrypted));
+4 -3
View File
@@ -130,9 +130,10 @@ export type CopyVerdict =
/**
* What the copies with the message's Message-ID say. A send creates the
* message as a draft and the submission's onSuccessUpdateEmail files it in
* Sent without `$draft` in the same request, so a copy without `$draft` went
* out (or is held for a scheduled time). Copies still marked `$draft` are
* message as a draft (in Drafts, or in Sent when the account has no Drafts
* folder) and only the submission's onSuccessUpdateEmail takes `$draft` off
* (and files it in Sent) in the same request, so a copy without `$draft`
* went out (or is held for a scheduled time). Copies still marked `$draft` are
* the autosaved draft (`draftId`, kept until the send succeeds) and copies
* a refused submission left behind with its answer lost: those go before
* the message is sent again.
+49
View File
@@ -391,6 +391,55 @@ describe('auth-store', () => {
expect(useAccountStore.getState().getAccountById('acc-1')?.needsSignIn).toBeFalsy();
});
// The network coming back (App.tsx) retries the session while the
// start from the saved session is still checking it: the retry used to
// load the account beside the check, and check the server while the
// start's reads were still landing.
it('waits for the check of the saved session when the network comes back during it', async () => {
registerAccount();
mockLoadFromCache.mockResolvedValueOnce(true);
const check = deferred<unknown>();
mockRevalidate.mockReturnValueOnce(check.promise);
await useAuthStore.getState().restoreSession();
let retried: boolean | undefined;
const retry = useAuthStore.getState().retrySession().then((ok) => { retried = ok; });
await new Promise((r) => setTimeout(r, 10));
expect(mockLoadAccount).not.toHaveBeenCalled();
expect(retried).toBeUndefined();
check.resolve({ apiUrl: 'https://mail.example.com/jmap/' });
await retry;
expect(retried).toBe(true);
expect(useAuthStore.getState().session).not.toBeNull();
expect(mockLoadAccount).not.toHaveBeenCalled();
expect(mockRevalidate).toHaveBeenCalledTimes(1);
expect((resume.mock.calls as unknown[][]).filter(([id]) => id === 'acc-1')).toHaveLength(1);
});
it('loads the account once the check of the saved session failed without network', async () => {
registerAccount();
mockLoadFromCache.mockResolvedValueOnce(true);
const check = deferred<unknown>();
mockRevalidate.mockReturnValueOnce(check.promise);
await useAuthStore.getState().restoreSession();
mockLoadAccount.mockResolvedValueOnce(true);
const retry = useAuthStore.getState().retrySession();
await new Promise((r) => setTimeout(r, 10));
expect(mockLoadAccount).not.toHaveBeenCalled();
check.reject(new NetworkError('offline'));
expect(await retry).toBe(true);
expect(mockLoadAccount).toHaveBeenCalledTimes(1);
expect(useAuthStore.getState().session).not.toBeNull();
// The start is confirmed: a failed switch away does not check it again.
mockLoadAccount.mockRejectedValueOnce(new NetworkError('offline'));
useAccountStore.setState({ accounts: [...useAccountStore.getState().accounts, account('acc-2', false)] });
await useAuthStore.getState().switchAccount('acc-2');
expect(mockRevalidate).toHaveBeenCalledTimes(1);
});
// A switch that fails puts the account back. Its saved session was
// never confirmed (the switch overtook the check), so the check runs
// again, and the outbox waits for it.
@@ -25,8 +25,10 @@ vi.mock('../../api/email', () => ({
}));
const runQueuedSend = vi.fn(async (..._a: any[]): Promise<any> => ({ kind: 'sent', result: { scheduled: false } }));
const wasQueuedSendSent = vi.fn(async (..._a: any[]) => false);
vi.mock('../../api/queued-send', () => ({
runQueuedSend: (...a: any[]) => runQueuedSend(...a),
wasQueuedSendSent: (...a: any[]) => wasQueuedSendSent(...a),
}));
vi.mock('../email-store', () => ({
useEmailStore: { getState: () => ({ mailboxes: [{ id: 'sent', role: 'sent' }] }) },
@@ -139,6 +141,37 @@ describe('offline send queue', () => {
expect(runQueuedSend).toHaveBeenCalledTimes(1);
});
it('sends a message queued while another one is on its way', async () => {
let release!: () => void;
runQueuedSend.mockImplementationOnce(() => new Promise((r) => { release = () => r({ kind: 'sent', result: {} }); }));
useNetworkStore.setState({ online: true, connected: true });
useOutboxStore.getState().enqueueSend(payload('first'));
await flushed();
expect(runQueuedSend).toHaveBeenCalledTimes(1);
useOutboxStore.getState().enqueueSend(payload('second'));
release();
await vi.waitFor(() => expect(useOutboxStore.getState().sends).toEqual([]));
expect(runQueuedSend.mock.calls.map(([s]) => s.payload.outgoing.subject)).toEqual(['first', 'second']);
});
it('sends a message retried while another one is on its way', async () => {
useNetworkStore.setState({ online: true, connected: true });
runQueuedSend.mockRejectedValueOnce(new Error('Invalid envelope'));
const failed = useOutboxStore.getState().enqueueSend(payload('failed'))!;
await vi.waitFor(() => expect(useOutboxStore.getState().sends[0]?.status).toBe('failed'));
let release!: () => void;
runQueuedSend.mockImplementationOnce(() => new Promise((r) => { release = () => r({ kind: 'sent', result: {} }); }));
useOutboxStore.getState().enqueueSend(payload('other'));
await flushed();
expect(runQueuedSend).toHaveBeenCalledTimes(2);
const retried = useOutboxStore.getState().retrySend(failed);
release();
await retried;
await vi.waitFor(() => expect(useOutboxStore.getState().sends).toEqual([]));
expect(runQueuedSend.mock.calls.map(([s]) => s.payload.outgoing.subject)).toEqual(['failed', 'other', 'failed']);
});
it('parks a refused message with its reason and leaves it until the user tries again', async () => {
useNetworkStore.setState({ online: true, connected: true });
runQueuedSend.mockRejectedValueOnce(new Error('Invalid envelope'));
@@ -154,6 +187,39 @@ describe('offline send queue', () => {
expect(useOutboxStore.getState().sends).toEqual([]);
});
it('says so when a message fails after too many unconfirmed attempts, as for a refused one', async () => {
useNetworkStore.setState({ online: true, connected: true });
runQueuedSend.mockRejectedValueOnce(new Error('Invalid envelope'));
useOutboxStore.getState().enqueueSend(payload('refused'));
await flushed();
await flushed();
expect(toast.error).toHaveBeenCalledTimes(1);
expect(toast.error.mock.calls[0][1]).toEqual({ message: 'refused: Invalid envelope' });
toast.error.mockClear();
const id = useOutboxStore.getState().enqueueSend(payload('unconfirmed'))!;
useOutboxStore.setState({ sends: useOutboxStore.getState().sends.map((s) => (s.id === id ? { ...s, attempts: 2 } : s)) });
runQueuedSend.mockRejectedValueOnce(named('SendUnconfirmedError', 'Send confirmation was not received'));
useOutboxStore.setState({ flushing: false });
await useOutboxStore.getState().flush();
await flushed();
expect(useOutboxStore.getState().sends.find((s) => s.id === id)).toMatchObject({ status: 'failed', attempts: 3 });
expect(toast.error).toHaveBeenCalledTimes(1);
expect(toast.error.mock.calls[0]).toEqual([
'A message in the Outbox could not be sent', { message: 'unconfirmed: Send confirmation was not received' },
]);
});
it('does not report an unconfirmed attempt that will be tried again', async () => {
useNetworkStore.setState({ online: true, connected: true });
runQueuedSend.mockRejectedValueOnce(named('SendUnconfirmedError', 'Send confirmation was not received'));
useOutboxStore.getState().enqueueSend(payload());
await flushed();
await flushed();
expect(useOutboxStore.getState().sends[0]).toMatchObject({ status: 'queued', attempts: 1 });
expect(toast.error).not.toHaveBeenCalled();
});
it('stops after an expired session without failing the message', async () => {
useNetworkStore.setState({ online: true, connected: true });
runQueuedSend.mockRejectedValueOnce(named('AuthenticationError', 'Session expired'));
@@ -188,6 +254,38 @@ describe('offline send queue', () => {
expect(useOutboxStore.getState().sends[0]).toMatchObject({ id: again, status: 'failed' });
});
it('does not turn a message an earlier attempt sent into a draft, and says it was sent', async () => {
const id = useOutboxStore.getState().enqueueSend(payload('went-out'), { maybeSent: true })!;
wasQueuedSendSent.mockResolvedValueOnce(true);
expect(await useOutboxStore.getState().cancelSend(id, { keepAsDraft: true })).toBe(true);
expect(wasQueuedSendSent).toHaveBeenCalledWith(expect.objectContaining({ id, maybeSent: true }), [{ id: 'sent', role: 'sent' }]);
expect(saveDraftVersion).not.toHaveBeenCalled();
expect(useOutboxStore.getState().sends).toEqual([]);
await flushed();
expect(toast.info).toHaveBeenCalledWith('This message was already sent', { message: 'went-out' });
expect(patchKeywordsForEmails).toHaveBeenCalledWith(['orig-1'], { $answered: true }, undefined);
});
it('moves a marked message to Drafts when no earlier attempt sent it, and looks only for marked ones', async () => {
const marked = useOutboxStore.getState().enqueueSend(payload('marked'), { maybeSent: true })!;
expect(await useOutboxStore.getState().cancelSend(marked, { keepAsDraft: true })).toBe(true);
expect(wasQueuedSendSent).toHaveBeenCalledTimes(1);
expect(saveDraftVersion).toHaveBeenCalledTimes(1);
const plain = useOutboxStore.getState().enqueueSend(payload('plain'))!;
expect(await useOutboxStore.getState().cancelSend(plain, { keepAsDraft: true })).toBe(true);
expect(wasQueuedSendSent).toHaveBeenCalledTimes(1);
expect(saveDraftVersion).toHaveBeenCalledTimes(2);
});
it('keeps a marked message in the Outbox when the lookup fails', async () => {
const id = useOutboxStore.getState().enqueueSend(payload('unknown'), { maybeSent: true })!;
wasQueuedSendSent.mockRejectedValueOnce(named('NetworkError', 'Network request failed'));
expect(await useOutboxStore.getState().cancelSend(id, { keepAsDraft: true })).toBe(false);
expect(saveDraftVersion).not.toHaveBeenCalled();
expect(useOutboxStore.getState().sends[0]).toMatchObject({ id, status: 'failed' });
});
it('drops the queued messages with the rest when the server behind the account changed', async () => {
useOutboxStore.getState().enqueueSend(payload());
expect(await useOutboxStore.getState().clearAccount(ACCOUNT)).toBe(1);
@@ -44,6 +44,54 @@ describe('explicitSettings', () => {
});
});
describe('settings saved before a default changed', () => {
// Older versions saved the full snapshot, so senderFavicons: true (the old
// default) sits in it whether or not the user ever turned it on.
it('does not count an old default as chosen', () => {
expect(legacyExplicitSettings({ senderFavicons: true, showBirthdayCalendar: true, density: 'compact' })).toEqual(['density']);
});
it('moves an old default to the new one on upgrade, and lets a policy default apply', async () => {
await AsyncStorage.setItem('webmail:settings:v1', JSON.stringify({
senderFavicons: true,
includeGroupInUnified: false,
autoSelectReplyIdentity: true,
showBirthdayCalendar: true,
density: 'compact',
}));
await useSettingsStore.getState().hydrate();
expect(useSettingsStore.getState()).toMatchObject({
senderFavicons: false,
includeGroupInUnified: true,
autoSelectReplyIdentity: false,
showBirthdayCalendar: false,
density: 'compact',
explicitSettings: ['density'],
});
resetPolicyRulesForTests();
await AsyncStorage.setItem('webmail:settings:v1', JSON.stringify({ senderFavicons: true, density: 'compact' }));
useSettingsStore.setState({ hydrated: false });
useSettingsStore.getState().applyPolicyRules(rules({ defaults: { senderFavicons: true, density: 'comfortable' } }));
await useSettingsStore.getState().hydrate();
// The organisation's default reaches the setting nobody chose.
expect(useSettingsStore.getState()).toMatchObject({ senderFavicons: true, density: 'compact', explicitSettings: ['density'] });
});
it('keeps settings saved with explicitSettings as they are', async () => {
await AsyncStorage.setItem('webmail:settings:v1', JSON.stringify({ senderFavicons: true, explicitSettings: ['senderFavicons'] }));
await useSettingsStore.getState().hydrate();
expect(useSettingsStore.getState()).toMatchObject({ senderFavicons: true, explicitSettings: ['senderFavicons'] });
});
it('treats an old default in an imported file the same way', () => {
useSettingsStore.setState({ hydrated: true });
expect(useSettingsStore.getState().importSettings(JSON.stringify({ senderFavicons: true, dateFormat: 'full' }))).toBe(true);
expect(useSettingsStore.getState()).toMatchObject({ senderFavicons: false, dateFormat: 'full' });
expect(useSettingsStore.getState().explicitSettings).not.toContain('senderFavicons');
});
});
describe('applyPolicyRules', () => {
it('does not write the defaults over stored settings it has not read yet', async () => {
await AsyncStorage.setItem('webmail:settings:v1', JSON.stringify({ theme: 'dark', explicitSettings: ['theme'] }));
+47 -3
View File
@@ -323,6 +323,26 @@ let restoreUnderWay: Promise<void> | null = null;
// connect, until the server confirms it (revalidateRestoredSession). Its
// session is not live yet: its server was not checked and its outbox waits.
let unconfirmedStart: string | null = null;
/** The reads that start sent with the saved session. */
let unconfirmedStartReads: { accountId: string; promise: Promise<void> } | null = null;
/**
* The check of the saved session under way. A session retry (the network
* coming back) during a start from the saved session waits for it instead of
* loading the account beside it.
*/
let revalidation: { accountId: string; promise: Promise<void> } | null = null;
function readsOfStart(accountId: string): Promise<void> | undefined {
return unconfirmedStartReads?.accountId === accountId ? unconfirmedStartReads.promise : undefined;
}
function startRevalidation(accountId: string, reads?: Promise<void>): Promise<void> {
const promise: Promise<void> = revalidateRestoredSession(accountId, reads).finally(() => {
if (revalidation?.promise === promise) revalidation = null;
});
revalidation = { accountId, promise };
return promise;
}
async function switchesSettled(): Promise<void> {
while (pendingSwitch) await pendingSwitch.settled;
@@ -627,6 +647,7 @@ function applyConnectedState(
// A session from the server, or a start from the saved one, which sets
// this again right after.
unconfirmedStart = null;
unconfirmedStartReads = null;
}
export const useAuthStore = create<AuthState>((set, get) => ({
@@ -1057,7 +1078,7 @@ export const useAuthStore = create<AuthState>((set, get) => ({
// still in the snapshot taken then, and its server was checked.
jmapClient.restoreSnapshot(origin.client);
if (origin.live) refetchFeatureStores(accountId);
else if (unconfirmedStart === accountId) void revalidateRestoredSession(accountId);
else if (unconfirmedStart === accountId) void startRevalidation(accountId);
set({ isLoading: false });
return;
}
@@ -1072,7 +1093,7 @@ export const useAuthStore = create<AuthState>((set, get) => ({
if (origin.accountId && origin.live) resumeOutbox(origin.accountId);
// A start from the saved session whose check this switch overtook.
else if (origin.accountId && unconfirmedStart === origin.accountId) {
void revalidateRestoredSession(origin.accountId);
void startRevalidation(origin.accountId);
}
};
try {
@@ -1245,7 +1266,8 @@ export const useAuthStore = create<AuthState>((set, get) => ({
unconfirmedStart = target.id;
perfMark('restore-done', { cached: true });
const reads = reloadFeatureStores();
void revalidateRestoredSession(target.id, reads);
unconfirmedStartReads = { accountId: target.id, promise: reads };
void startRevalidation(target.id, reads);
void syncAccountDisplayName(target.id);
return true;
}
@@ -1358,6 +1380,20 @@ export const useAuthStore = create<AuthState>((set, get) => ({
if (!target) return false;
// The server refused its sign-in: only a new one brings it back.
if (target.needsSignIn) return false;
// A start from the saved session: its check brings the session up, after
// the reads that start sent. Loading the account beside it checked the
// server while those reads were still landing. Only a check that ended
// without network leaves the load below to try.
if (unconfirmedStart === activeAccountId) {
const running = revalidation?.accountId === activeAccountId
? revalidation.promise
: startRevalidation(activeAccountId, readsOfStart(activeAccountId));
await running;
if (!isCurrent() || get().activeAccountId !== activeAccountId) return false;
if (get().session) return true;
if (get().reauth || unconfirmedStart === activeAccountId) return false;
}
const startReads = readsOfStart(activeAccountId);
try {
const ok = await jmapClient.loadAccount(activeAccountId);
if (!ok || !isCurrent()) return false;
@@ -1369,6 +1405,13 @@ export const useAuthStore = create<AuthState>((set, get) => ({
});
const fresh = jmapClient.currentSession!;
followStoredServer(activeAccountId);
if (startReads) {
// What the start read from the saved session lands before a server
// that was replaced drops the caches.
const recorded = await readServerFingerprint(activeAccountId).catch(() => null);
if (recorded !== sessionFingerprint(fresh)) await startReads;
if (!isCurrent()) return false;
}
await checkServerChange(activeAccountId, fresh);
if (!isCurrent()) return false;
applyConnectedState(set, fresh, target.serverUrl, target.username, activeAccountId);
@@ -1449,6 +1492,7 @@ async function revalidateRestoredSession(accountId: string, reads?: Promise<void
const live = jmapClient.currentSession;
if (!isCurrent() || !live) return;
unconfirmedStart = null;
unconfirmedStartReads = null;
useAccountStore.getState().updateAccount(accountId, {
isConnected: true,
hasError: false,
+48 -5
View File
@@ -258,6 +258,12 @@ function opFamily(op: OutboxOp): string {
let retryTimer: ReturnType<typeof setTimeout> | null = null;
let retryAttempt = 0;
/**
* A flush was asked for while one ran (a message queued or retried then):
* the running one works from lists it took when it started, so another
* follows it.
*/
let flushAgain = false;
// Reads of a bucket from storage. A read still running when the account
// changes or its queue is cleared must not put the old entries back, so
@@ -436,7 +442,11 @@ export const useOutboxStore = create<OutboxState>((set, get) => ({
pendingForEmail: (emailId) => get().entries.filter((e) => e.op.emailId === emailId),
flush: async () => {
if (get().flushing || get().paused || !get().resumed || !get().hydrated) return;
if (get().flushing) {
flushAgain = true;
return;
}
if (get().paused || !get().resumed || !get().hydrated) return;
const accountId = get().activeAccountId;
if (!accountId) return;
if (get().entries.length === 0 && !hasWaitingSends()) return;
@@ -445,6 +455,7 @@ export const useOutboxStore = create<OutboxState>((set, get) => ({
if (!useNetworkStore.getState().online) return;
set({ flushing: true });
flushAgain = false;
let brokeTransient = false;
let dropped = false;
// An entry that took on a later edit while its op was out runs again.
@@ -509,7 +520,7 @@ export const useOutboxStore = create<OutboxState>((set, get) => ({
set({ flushing: false });
}
if (brokeTransient && (get().entries.length > 0 || hasWaitingSends())) scheduleRetry();
else if (changedInFlight) void get().flush();
else if (changedInFlight || flushAgain) void get().flush();
if (dropped) void refreshAfterDrop();
},
@@ -562,6 +573,22 @@ export const useOutboxStore = create<OutboxState>((set, get) => ({
if (!opts?.keepAsDraft) return true;
const p = send.payload;
try {
// An attempt that may have reached the server is looked for first, as
// before a resend: a message that went out must not come back as a
// draft.
if (send.maybeSent) {
const [{ wasQueuedSendSent }, { useEmailStore }] = await Promise.all([
import('../api/queued-send'),
import('./email-store'),
]);
if (await wasQueuedSendSent(send, useEmailStore.getState().mailboxes)) {
if (p.original) {
patchKeywordsForEmails([p.original.emailId], { [p.original.keyword]: true }, p.original.accountId).catch(() => undefined);
}
void notifyAlreadySent(send);
return true;
}
}
if (!p.draftsMailboxId) throw new Error('No Drafts folder');
const { saveDraftVersion } = await import('../api/email');
await saveDraftVersion(p.outgoing, p.draftsMailboxId, p.draftId, undefined, p.draftAccountId);
@@ -690,14 +717,17 @@ async function flushSends(accountId: string): Promise<'done' | 'transient'> {
} catch (err) {
const kind = classifySendFailure(err, isTransientNetworkError);
const message = err instanceof Error ? err.message : String(err);
updateSend(accountId, afterFailure(started, kind, message));
const next = afterFailure(started, kind, message);
updateSend(accountId, next);
// Refused, or unconfirmed too often: it waits for the user now, and
// nothing retries it, so say so whatever the reason.
if (next.status === 'failed') void notifyQueuedSendFailed(started, message);
if (kind === 'auth') {
useOutboxStore.setState({ paused: true });
break;
}
if (kind === 'offline' || kind === 'unconfirmed') return 'transient';
// Refused: this one waits for the user; the others go on.
void notifyQueuedSendFailed(started, message);
// Refused: the others go on.
}
}
return 'done';
@@ -729,6 +759,19 @@ async function afterQueuedSend(send: QueuedSend, outcome: QueuedSendOutcomeLike)
}
}
/** Taking back a message found to have gone out already: nothing became a draft. */
async function notifyAlreadySent(send: QueuedSend): Promise<void> {
try {
const [{ toast }, { useLocaleStore }] = await Promise.all([import('./toast-store'), import('./locale-store')]);
const t = useLocaleStore.getState().t;
toast.info(t('outbox.already_sent', 'This message was already sent'), {
message: send.payload.outgoing.subject || undefined,
});
} catch (err) {
console.warn('[outbox] could not report an already sent message', err);
}
}
async function notifyQueuedSendFailed(send: QueuedSend, message: string): Promise<void> {
try {
const [{ toast }, { useLocaleStore }] = await Promise.all([import('./toast-store'), import('./locale-store')]);
+48 -7
View File
@@ -858,17 +858,54 @@ function policyContext(explicit: readonly string[]): PolicyPatchContext {
return { builtIn: POLICY_BUILT_IN, explicit: new Set(explicit), isValid: isValidPolicyValue };
}
/**
* Defaults an earlier version had, for the settings whose default changed
* since. Versions before explicitSettings saved the whole snapshot, defaults
* included, so a value equal to the old default was most likely never
* picked: senderFavicons was on until sender domains stopped going to the
* favicon service by default; the other three changed when the defaults were
* aligned with the webmail (0.1.59).
*/
const LEGACY_DEFAULTS: Readonly<Partial<Record<keyof PersistedSettings, unknown>>> = {
senderFavicons: true,
includeGroupInUnified: false,
autoSelectReplyIdentity: true,
showBirthdayCalendar: true,
};
function isLegacyDefault(source: Record<string, unknown>, key: string): boolean {
return key in LEGACY_DEFAULTS && source[key] === LEGACY_DEFAULTS[key as keyof PersistedSettings];
}
/**
* Settings from before explicitSettings existed: a value other than the
* build default can only have come from the user, so it counts as chosen
* (webmail legacyExplicitSettings).
* (webmail legacyExplicitSettings). A value equal to that setting's old
* default doesn't (webmail migrateNotificationSoundDefault).
*/
export function legacyExplicitSettings(source: Record<string, unknown>): string[] {
return Object.keys(source).filter(
(key) => isPolicySettingKey(key) && source[key] !== undefined && source[key] !== POLICY_BUILT_IN[key],
(key) =>
isPolicySettingKey(key) &&
source[key] !== undefined &&
source[key] !== POLICY_BUILT_IN[key] &&
!isLegacyDefault(source, key),
);
}
/**
* Moves a setting still at its old default, in settings from before
* explicitSettings existed, to the current default: the user never picked
* it (webmail migrateNotificationSoundDefault, migrateTintListRowsDefault).
*/
export function migrateLegacyDefaults<T extends Record<string, unknown>>(source: T): T {
const out: Record<string, unknown> = { ...source };
for (const key of Object.keys(LEGACY_DEFAULTS)) {
if (isLegacyDefault(out, key)) out[key] = DEFAULT_PERSISTED[key as keyof PersistedSettings];
}
return out as T;
}
/** Whether the organisation policy pins `key`, so the user can't change it. */
export function isSettingLockedByPolicy(key: string): boolean {
return isLockedSetting(policyRules, key);
@@ -942,9 +979,10 @@ export const useSettingsStore = create<SettingsState>((set, get) => ({
const raw = await AsyncStorage.getItem(STORAGE_KEY);
if (raw) {
const parsed = JSON.parse(raw) as Partial<PersistedSettings>;
const merged = mergeWithDefaults(parsed);
let merged = mergeWithDefaults(parsed);
if (!Array.isArray(parsed.explicitSettings)) {
merged.explicitSettings = legacyExplicitSettings(merged as unknown as Record<string, unknown>);
merged = migrateLegacyDefaults(merged as unknown as Record<string, unknown>) as unknown as PersistedSettings;
}
// The policy may have arrived (from its cache) before the settings.
const patch = currentPolicyPatch(merged);
@@ -1089,12 +1127,15 @@ export const useSettingsStore = create<SettingsState>((set, get) => ({
return false;
}
if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return false;
const incoming = fromExportShape(parsed as Record<string, unknown>);
let incoming = fromExportShape(parsed as Record<string, unknown>);
// A file without explicitSettings (older clients): what it sets to
// something other than the default was chosen.
const chosen = Array.isArray(incoming.explicitSettings)
? incoming.explicitSettings.filter((key): key is string => typeof key === 'string')
// something other than the default was chosen, and what is still at an
// old default moves to the current one.
const legacy = !Array.isArray(incoming.explicitSettings);
const chosen = !legacy
? (incoming.explicitSettings as unknown[]).filter((key): key is string => typeof key === 'string')
: legacyExplicitSettings(incoming as Record<string, unknown>);
if (legacy) incoming = migrateLegacyDefaults(incoming as Record<string, unknown>) as typeof incoming;
// Validate against the current state so keys absent from the file keep
// their value instead of snapping back to the default.
const merged = mergeWithDefaults({