Skip to content
This repository was archived by the owner on May 15, 2024. It is now read-only.

Implement prefersEphemeralWebBrowserSession Fixes #1712#1837

Merged
Redth merged 9 commits intomainfrom
gh-1712
Jul 12, 2021
Merged

Implement prefersEphemeralWebBrowserSession Fixes #1712#1837
Redth merged 9 commits intomainfrom
gh-1712

Conversation

@jamesmontemagno
Copy link
Collaborator

Fixes #1712

@jamesmontemagno jamesmontemagno requested review from Redth and mattleibow and removed request for mattleibow June 24, 2021 21:26
Copy link
Member

@dalexsoto dalexsoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look right to me but not my area of expertise @mandel-macaque could you also have a look? Thanks!

mattleibow
mattleibow previously approved these changes Jun 24, 2021
Copy link

@mandel-macaque mandel-macaque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in the surface, I fear that server side set cookies are going to be a problem,

Comment on lines +91 to +101
if (prefersEphemeralWebBrowserSession)
{
NSUrlCache.SharedCache.RemoveAllCachedResponses();
WKWebsiteDataStore.DefaultDataStore.HttpCookieStore.GetAllCookies((cookies) =>
{
foreach (var cookie in cookies)
{
WKWebsiteDataStore.DefaultDataStore.HttpCookieStore.DeleteCookie(cookie, null);
}
});
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not believe that the fix is that easy. AFAIK the WKWebsiteDataStore.DefaultDataStore.HttpCookieStore returns a WKHttpCookieStore that will have a shred cookies for the process pool, this object does not allow to remove the cookies that have been set server side, at least the last time I checked, this means that all those cookies will be present as long as you are still using the same WKProcess.

Have you tested this via a cookie set from the server side? I think that you will need to remove the cookies AND create a new WKProcess.

Another way to do this is to use the ephemeral setting in the NSUrlSessionHanlder, which will not save the cookies and won't make you do this dance. Do you have a pointer in the code where you are making the call. The Ephemeral setting by the OS might be a cleaner way.

PS: Sorry, I don't know the code base enough, but I do know that if you do not change the WKProcess the cookies are there.

@@ -1,3 +1,7 @@
using Foundation;

[assembly: System.Reflection.AssemblyMetadata("IsTrimmable", "True")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's introduce this in a different PR in the future.

Suggested change
[assembly: System.Reflection.AssemblyMetadata("IsTrimmable", "True")]

@jamesmontemagno jamesmontemagno added this to the 1.7.0 milestone Jun 28, 2021
@Redth Redth merged commit 12177bc into main Jul 12, 2021
@Redth Redth deleted the gh-1712 branch July 12, 2021 14:04
Redth added a commit to dotnet/maui that referenced this pull request Jul 12, 2021
mattleibow pushed a commit to dotnet/maui that referenced this pull request Jul 20, 2021
maxkatz6 pushed a commit to AvaloniaUI/Avalonia.Essentials that referenced this pull request May 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WebAuthenticator iOS - Sign out does not delete the cookies and we are unable to sign in as a different user

5 participants