search

SAP / gigya-swift-sdk

SAP CDC (Gigya) swift sdk for mobile
Apache License 2.0
25 stars 17 forks source link

Clear Cookies only on Logout #64

Closed vvbutko closed 1 year ago

vvbutko commented 1 year ago

Summary

This PR modifies the cookie clearing behavior to make it more consistent with the behaviuor observed in the Gigya Android SDK. Specifically, it removes the unexpected cookie clearing during the implicit initialization when a session has expired but the user has not logged out.

Motivation

We have encountered unexpected cookie clearing from the Gigya SDK during implicit initialization. This situation arises from the SessionService.clearCookies() method, which is invoked from SessionService.startSessionCountdownTimerIfNeeded(), when if !session.isValid() is true. The session is invalid because it has expired (GigyaSession.isValid() returns false).

In our view, device cookie clearing should occur upon logout, not when the session expires. This logic aligns with the Android SDK's behaviour, which we also use and where we do not encounter any issues. A comparative analysis of the iOS and Android SDKs has revealed this discrepancy.

It's worth noting that utilizing Gigya.sharedInstance().setClearCookies(to: false) does not solve the issue, as it only alters the behaviour after explicit init completion, while the unexpected clearing takes place during the "implicit" init.

Modifications

Below are the suggested modifications that are implemented in this PR, along with references to the Android SDK for context.

1. Remove cookie clearing from session clearing.

The SessionService.clearSession() method is invoked when the session expires and should not clear the cookies.

Android code reference: SessionService.java line 282. No cookie clearing is present during session clearing.

2. Implement cookie clearing upon user logout.

The BusinessApiService.logOut(completion:) method should explicitly clear the cookies. Previously, logout triggered sessionService.clear(), which cleared cookies. However, this does not allow the separation of cookie clearing from session expiration. By moving the cookie clearing call directly to BusinessApiService, we enforce it only upon user logout, eliminating the unexpected behaviour.

Android code reference: SessionService.java line 294. The only instances where cookies can be cleared are from clearCookiesOnLogout(), which in turn is invoked from only two places:

  1. Gigya.java line 395 - for logout.
  2. GigyaWebBridge.java line 353 - for web-bridge logout.

For the iOS SDK, both of these logout flows invoke BusinessApiService.logOut(completion:), ensuring cookies will be cleared in all logout scenarios as expected.

vvbutko commented 1 year ago

Sorry. I didn't know deleting a forked repo closes the PR. I'll create a new PR shortly.