-
Notifications
You must be signed in to change notification settings - Fork 83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement Android's removeSessionCookies method #145
Implement Android's removeSessionCookies method #145
Conversation
Bump, I'd like to get this PR merged in if possible. Let me know if there's any concerns. |
public void removeSessionCookies(Promise promise) { | ||
try { | ||
getCookieManager().removeSessionCookies(new ValueCallback<Boolean>() { | ||
@Override | ||
public void onReceiveValue(Boolean data) { | ||
promise.resolve(data); | ||
} | ||
}); | ||
} catch (Exception e) { | ||
promise.reject(e); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to flush here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not flush, sync*
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I've seen so far, we didn't have to do a flush or sync or anything else to get fetch or a webview to get rid of these session cookies. We've just been using this call and the next fetch we do results in the session cookies no longer being there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sg! lets ship it
# [6.2.0](v6.1.0...v6.2.0) (2022-04-18) ### Features * **Android:** Implement Android's removeSessionCookies method ([#145](#145)) ([b459739](b459739))
🎉 This PR is included in version 6.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Awesome, thanks! |
Description
I'm submitting a PR to implement the Android CookieManager removeSessionCookies method. Docs on what this does exactly are at the Android CookieManager documentation. As a quick description, this lets developers remove all cookies that are session cookies. These are cookies with no Expires attribute.
We use Rails with the Devise gem to handle authentication. Rails sets a session cookie with no Expires attribute and a remember user cookie with a desired expiry. Currently, the session cookie will never be removed as per how Android operates so the remember period is effectively ignored on Android. So implementing this method will give RN developers a way of controlling whether they want to clear these out. As an example, we call this prior to checking with an API call if the user is still logged in during the startup of the app.
Type of change
How Has This Been Tested?
I have used MITMProxy with an Android API 30 emulator and a API 28 LG G6 phone to verify that session cookies can be removed prior to performing a
fetch
call in RN and it removes the session cookies from the next call correctly. This should also work for Android webviews as well.