fix(ios): use persistent WKWebsiteDataStore so cookies survive cold launch#140
Open
sawan-webmavens wants to merge 1 commit into
Open
fix(ios): use persistent WKWebsiteDataStore so cookies survive cold launch#140sawan-webmavens wants to merge 1 commit into
sawan-webmavens wants to merge 1 commit into
Conversation
…aunch WKWebsiteDataStore.nonPersistent() is in-memory storage. The WKWebView starts with an empty cookie jar on every cold launch, which wipes the laravel_session cookie and makes the user appear signed-out even though their session file is still on disk. Android already uses persistent storage via CookieManager.getInstance(), so this aligns iOS with Android behavior. Refs NativePHP#139
Member
|
I think it's right to have consistent behaviour across both platforms 👍🏼 My caveat is that I think cookies as a mechanism for determining authenticated state is kind of "wrong" (convenient, but not as secure as secure storage) in this context and the fact that Android supports it actually suggests, imo, that it's Android that should change to align with iOS. Just my opinion tho and some history on why I chose for iOS cookie jars to be in-memory only in the first place. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WKWebsiteDataStore.nonPersistent()to.default()so cookies persist across cold launches.CookieManager.getInstance().Why
WKWebsiteDataStore.nonPersistent()is in-memory storage — the WKWebView starts with an empty cookie jar on every cold launch. For Laravel apps this wipes thelaravel_sessioncookie, so even though the session file is still on disk instorage/framework/sessions/, Laravel can't find it. The user appears signed-out, and anysession(...)state (auth user, flash messages, dismissed prompts) is reset.Android already does the right thing —
CookieManager.getInstance()is persistent and shared by default. Same Laravel code, two different behaviors. This patch brings iOS in line.Risk
WKWebsiteDataStore.default()is the shared, app-wide persistent jar. Not an issue for NativePHP apps (they own their entire WebView), but flagging in case reviewers want it on the radar.Test plan
auth()->check()istrueon next cold launch (wasfalsebefore this change).session(...)state survives cold launch.Notes
Refs #139, which raised the open question of unconditional flip vs. config option. This PR is the unconditional flip — happy to amend to a config option (e.g.
'cookies' => 'persistent'|'ephemeral'defaulting to persistent) if you'd rather preserve the current behavior as an opt-in.