fixed android logout

[svzi]

As mentioned in #97 there are currently some issues with the logout functionality. We discovered them only on Android, so this fix is only implementing a solution for Android. Our project uses SalesForce OAuth2 provider.

We found that our users had problems with logout whenever they did not use their system's default browser. For example, if they had configured the Brave browser or Firefox as their default browser. In these cases we were not able to delete the session cookies. This is because this plugin always uses the system browser (usually Chrome), no matter what the user has set as default. But when we open a new browser window using Capacitor, the default browser configured by the user is used. And just not the system browser.

That's why we implemented the logout functionality for Android and our users can now logout correctly on Android. No matter which browser they use and no matter which browser is default on their system.

I implemented the customization in such a way that no changes are necessary for existing applications. The code is fully backward compatible.

If you want to use the new logout functionality, you now have to pass the current accessToken as a second, optional, parameter to the logout(...) function (required by the Android plugin). If you do not pass the parameter, the plugin will behave as before.

I have adapted the documentation as good as possible. If it is incomplete or you want it to be different, please let me know.

[moberwasserlechner]

use "provide" instead of "provice"

[svzi]

Thanks for pointing that out. Fixed it.

[moberwasserlechner]

Thanks for this PR

[svzi]

You're welcome and I need to thank you for providing such an awesome plugin. 🏆

[Ssnipo]

I might be wrong but id_token parameter currently has no effect. I have printed the result of call.getData() and it's not there.

For tests purpose I forced id_token with this snippet:

// @ts-ignore
oAuth2AuthenticateOptions.id_token = _accessToken;

That will result into NullPointerException because the logoutUrl is not set in this method:
OAuth2Options buildAuthenticateOptions(JSObject callData)

Also setLogoutUrl(String logoutUrl) is not defined in OAuth2Options.java

Making these changes will end in this error that I could not solve:
java.lang.NullPointerException: Attempt to invoke virtual method 'android.net.Uri$Builder android.net.Uri.buildUpon()' on a null object reference

[zolakt]

Just realized the same thing as @Ssnipo. This can't possibly work. It seems to have been merged blindly without any testing.

For what it's worth, even if it didn't throw that last exception @Ssnipo has pointed out, I think this is just logically wrong:

EndSessionRequest endSessionRequest = new EndSessionRequest.Builder(config)
    .setIdTokenHint(idToken)
    .setPostLogoutRedirectUri(logoutUri)
    .build();

PostLogoutRedirectUri and LogoutUri are not the same thing.

Considering this has been reported over a year ago, is this lib even maintained anymore?

[moberwasserlechner]

@zolakt you are very welcome to help maintaining this lib. That's how OSS should work, considering the work for this lib was done in ones spare time.

[zolakt]

I'm not trying to criticise, I'm just asking if anyone is maintaining this.

Since, let's be honest, merging this PR was obviously done without any testing (from the contributor, or you), and it's doing more harm than good. It is just misleading, along with the docs, in making it seem like this should work, and it definitely doesn't. And the issues with this have been pointed out over a year ago. I'm not the only one that got confused, and spent time debugging. So please, at least fix the docs.

If I ever get the time, I'll try to help out. So far, I've resorted to the workaround to use the authenticate() method to do the logout. That at least works, and will have to do for now. I'm pointing other people to do the same, so they don't waste time

[svzi]

I feel really sorry about that my implementations didn't work as expected. I used it in a client project back then and it worked there. Probably I had more changes directly in the project that I forgot to check in, I doubt it, but it could be the case. It's too long ago to be sure about.

If I ever get the time, I'll try to help out.

That's exactly the point why no one has fixed it so far, I guess. Because we all feel so.

[moberwasserlechner]

@svzi I think your change back then was fine as well. You tested it and if there is another aspect to it, it simply is a bug or an improvement. So thanks for the PR back then.

@zolakt If you know whats wrong with the docs please at least make a PR for the docs. You have thought into the topic and I myself and others have not.

[zolakt]

Honestly I doubt that it ever worked, since there are multiple issues with it. It's more likely that not everything was pushed, back then. The id_token argument that was added isn't parsed properly. Even if it was, it's never used, since it's not included in OAuth2Options. The setter is missing there, and there is no code that would set it up. It's mentioned in this post #233 (comment). I've came to the same conclusion, and gave up on the same issue.

@moberwasserlechner Here you go #278
I removed all mentions of logout() from the examples, and added a section which points to the issue where workarounds are discussed.