chore: unified cookie storage - WPB-23404 by samwyndham · Pull Request #4596 · wireapp/wire-ios

samwyndham · 2026-04-21T07:27:26Z

WPB-23404 [iOS] Review cookie handling

Issue

My apologies. This PR has turned into something much bigger than I intended 🙏.

Currently the app has two versions of CookieStorage - the legacy obj-c ZMPersistentCookieStorage & the new CookieStorage. There are some issues with this such as the legacy version using a cache and the new one not... E.g. if the cookie is updated via the new cookie storage, this may not be reflected in the legacy cookie due to it's cache.

This PR attempts to unify the cookie storage mechanisms and make some other safety improvements to try to make everything work well in a concurrent and multiprocess environment. Specifically it:

Refactors CookieStorage making it a non actor and removing async methods so that it can be used from non async contexts. It's access it protected by a lock to ensure that storing, fetching & deleting are generally atomic operations.
Made CookieStorage act on all users not a single user. Eg. a call to fetchCookies() is now fetchCookies(userID: UUID). The reasoning for this is that 1. in some cases cookie storage is not cleanly tied to a user session, 2. There are currently some static methods on LegacyCookieStorage (e.g. testing that any cookie exists). These methods should move to CookieStorage eventually and it would be better to make them non static to allow for injection etc.
Moves in memory cache to CookieStorage and changes how it works. We now store an epoch as metadata to Keychain items when they are . When adding to the cache we also store the epoch. When fetching from the cache we compare an items epoch to that in the Keychain to determine if the cached item is still valid. This works across processes meaning that we can now uses caching in app extensions and an cookie update coming from an app extension would now not break the cache in the main app (I'm not sure this happens in practice but believe it could happen).
Refactored ZMPersistentCookieStorage into LegacyCookieStorage which now wraps CookieStorage and is a much simpler object. Interestingly I started this refactor by using Claude to convert ZMPersistentCookieStorage and its tests from obj-c to swift. This worked well.
Moved CookieStorageTests to the app test target so that I could write tests against the real keychain - I think this is worthwhile vs mocking because we can see that the behavior is really what we expect.
Removed the concept of cookie policies from the legacy store as in practice this was always NSHTTPCookieAcceptPolicyAlways.

This PR also fixes: https://wearezeta.atlassian.net/browse/WPB-24720

@johnxnguyen @netbe I accidently merged as dependent PR - 84eafe5 - into this before this was reviewed. This contains UI tests and has been independently reviewed. Feel free to skip over reviewing the UI test code.

Testing

General regression test logging in & out of multiple accounts, and making use of app extensions.

Checklist

Title contains a reference JIRA issue number like [WPB-XXX].
Description is filled and free of optional paragraphs.
Adds/updates automated tests.

This is dead code apart from in tests

This reverts commit fa6dc9b.

Account name should be unique per server. That is the assumption we are making when storing in the keychain so no point doing something different for the cache. As far as I understand it was only there for legacy migrations

…CookieStorage

This is a Swift version of ZMPersistentCookieStorage

This file isn't used # Conflicts: # WireNetwork/Sources/WireNetwork/Assembly.swift

…che:)

github-actions · 2026-04-21T09:15:20Z