SAP / gigya-android-sdk

SAP CDC (Gigya) android sdk for mobile
Apache License 2.0
19 stars 13 forks source link

External GoogleProviderWrapper does not perform logout #46

Closed josefgrunig closed 1 year ago

josefgrunig commented 1 year ago

When implementing the GoogleProviderWrapper in our application and performing a logout on the Gigya instance, the overridden logout method of the GoogleProviderWrapper is not being called. That causes to enter an inconsistent state that prevents from further logins using Google Sign In. Having a look at the IoCContainer logs we noticed that there was still an error while getting the Provider Class during the logout and in the source code we noticed some wrapper renaming from googleplus to google. Adding an empty provider named "GoogleplusProviderWrapper" fixes all the issues.

The logs without the GoogleplusProviderWrapper, while performing logout

<< SessionVerificationService *** stop:  >>
<< ProviderManager *** Error getting provider class name. Check root path >>
<< GigyaApiResponse *** ApiService: https://accounts.eu1.gigya.com/accounts.logout

The logs with the GoogleplusProviderWrapper added, while performing logout

<< SessionVerificationService *** stop:  >>
<< IoCContainer *** Trying to get: package.omitted.gigya.providers.GoogleplusProviderWrapper >>
<< IoCContainer *** Trying to create new instance for: package.omitted.gigya.providers.GoogleProviderWrapper >>
<< IoCContainer *** For constructor with params #: 1 >>
<< IoCContainer *** Getting required param: android.content.Context >>
<< IoCContainer *** Trying to get: android.content.Context >>
<< IoCContainer *** binding package.omitted.gigya.providers.GoogleProviderWrapper to instance (of type package.omitted.gigya.providers.GoogleProviderWrapper) >>

Step to reproduce:

  1. Perform a Google Social Login via ScreenSet
  2. Logout
  3. Try to log in again
tal-mi commented 1 year ago

Hi, looking into it. Planned update on Sunday. I will push it though. Tal

josefgrunig commented 1 year ago

Hi @tal-mi, having it on Sunday would keep us on track for a release later next week, thank you. If you need any help on reproducing this issue just let me know; I noticed that even using the GoogleplusProviderWrapper workaround, I found a case where it does not work. Here the steps that reproduce the issue:

  1. Perform a Google Social Login via ScreenSet
  2. Close the app
  3. Open the App
  4. Session is correctly maintained, We can see the profile information
  5. Logout (again the GoogleProviderWrapper logout method is not invoked)

I think there is something wrong in the getUsedSocialProviders() method skipping over the actually used providers.

tal-mi commented 1 year ago

I will have a deep dive into it. I will be moving into a solid solution without the use of a workaround. Thank you for the info.

tal-mi commented 1 year ago

Fixed the issue. Will be deployed in the upcoming version (upcoming days).

josefgrunig commented 1 year ago

Hi @tal-mi, Thank you for the fix, I will give it a try as soon as a release is published. When is it scheduled? I need to know because we need to go in submission with the fixes within mid of this week, or roll back all the changes. Thank you for the feedback.

tal-mi commented 1 year ago

The release will not be before Wednesday. We are currently waiting for approval. Fix has been pushed to develop branch if you want to observe.

josefgrunig commented 1 year ago

Hi @tal-mi, we have updated to v7.0.1, but the issue seems not gone. The behaviour has changed but still not correct. We have substituted the updated GoogleProviderWrapper, but still the overridden logout method is not being called leaving somehow the session with google sign in open. When we try to login again we see a "Server Error" message on the ScreenSet and logs say:

< ApiService *** GET_SDK_CONFIG with:
{
  "callId": "ed3f2... omitted...5b832",
  "errorCode": 500002,
  "errorMessage": "Server login error",
  "apiVersion": 2,
  "statusCode": 500,
  "statusReason": "Internal Server Error",
  "time": "2023-05-03T11:53:51.609Z",
  "ignoredParams": [
    {
      "paramName": "code",
      "warningCode": 403007,
      "message": "This parameter was not recognized as valid for this API method with your security credentials nor was it recognized as a standard Gigya control parameter."
    }
  ],
  "errorDetails": "500002 - Server login error"
}

Trying my previous workaround (using a clone of the GoogleProviderWrapper named GoogleplusProviderWrapper) I can logout correctly and perform a login properly.

tal-mi commented 1 year ago

Hi, Can u provide a snippet of code so I can review how you initiate the social login and logout flow? Using login method or screen sets? tested both with fix without issues. The Google plus name is deprecated and is set to be avoided in the sdk.

josefgrunig commented 1 year ago

We perform login via screen sets. Here we show the Login/Registration screenset:

HashMap<String, Object> params = new HashMap<>();
params.put("screenSet", getScreenSetName());
params.put("startScreen", "gigya-login-screen");
params.put("lang", getLanguage());
params.put("deviceType", "mobile");

Gigya.getInstance(GigyaAccount.class).showScreenSet(screenSetName, false, true, params, new GigyaPluginCallback<GigyaAccount>() {
    @Override
    public void onLogin(@NonNull GigyaAccount accountObj) {
        super.onLogin(accountObj);
    }

    @Override
    public void onCanceled() {
        super.onCanceled();
    }
});

Logout is performed calling the method on the SDK

Gigya.getInstance().logout();

tal-mi commented 1 year ago

Hi, The release was incorrect. Updated v7.0.1 with the correct fix.

josefgrunig commented 1 year ago

Hi @tal-mi, I confirm the issue has gone and login works flawlessly! Thank you for the support

tal-mi commented 1 year ago

Np. Next version i will be adding the option to set the custom path for the providers so they won't have to be under gigya.providers package. Cheers.

josefgrunig commented 1 year ago

Np. Next version i will be adding the option to set the custom path for the providers so they won't have to be under gigya.providers package. Cheers.

Just a note: it might be useful to write in the documentation to exclude those files from the minification process, otherwise reflection won't find the class even if at the right place. Thank you.