Skip to content
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

removeAll(forClient: ) does not work on update 2.7.3 #1341

Open
andrewcookies opened this issue Nov 6, 2018 · 20 comments
Open

removeAll(forClient: ) does not work on update 2.7.3 #1341

andrewcookies opened this issue Nov 6, 2018 · 20 comments
Assignees
Labels

Comments

@andrewcookies
Copy link

Hi,
I recently update ADAL from 2.6.2 to 2.7.3 and the first thing I noticed is that the
ADKeychainTokenCache.defaultKeychain().removeAlll(forClientId: client, error: nil) does not work and don't even catch any error., while everything is ok on 2.6.2 version.

How to solve this?

Thanks!

Andrea

@oldalton
Copy link
Member

oldalton commented Nov 6, 2018

Hi. When you say it doesn't work, can you please provide more details on the exact issue you're facing? Thanks.

@antrix1989
Copy link
Collaborator

Please make sure you provide a reference to ADAuthenticationError so you can get the error back if something failed:

ADAuthenticationError *error;
ADKeychainTokenCache.defaultKeychain().removeAlll(forClientId: client, error: &error);

@andrewcookies
Copy link
Author

Hi,
after call ADKeychainTokenCache.defaultKeychain().removeAll (...), the .allItems returns the client id anyway.
Moreover, the error = nil

Thanks

@antrix1989
Copy link
Collaborator

Could you please send your logs to this email [email protected]?

@nitaeduard
Copy link

Any update on this situation? I have been experiencing the same issues, using the same method.
The error is nil, but calling ADKeychainTokenCache.defaultKeychain().allItems(nil) will still return previously removed items.

@oldalton
Copy link
Member

oldalton commented Dec 6, 2018

@nitaeduard, we weren't able to reproduce it ourself. Would you be able to provide ADAL logs for us, so we can investigate further? Please see instructions on logging here. Please send logs to [email protected].

@oldalton
Copy link
Member

oldalton commented Dec 9, 2018

Alternatively to logs, you can execute following code and copy the output:

NSLog(@"All items %@", [[ADKeychainTokenCache defaultKeychainCache] allItems:nil]);
NSLog(@"Removal result %d for clientId %@", [[ADKeychainTokenCache defaultKeychainCache] removeAllForClientId:@"myclient_id" error:nil], @"myclient_id");
NSLog(@"All items %@", [[ADKeychainTokenCache defaultKeychainCache] allItems:nil]);

Note that you need to replace @"myclient_id" with your own client id.

@oldalton
Copy link
Member

Thanks for providing logs. Based on the logs, I believe it's the same issue as #1356, as it's dealing with ADFS. Will be fixed in ADAL 2.7.7 release, PR #1357

@oldalton
Copy link
Member

@AndreaGrantu, can you confirm that your issue also reproduces with ADFS authentication?

@andrewcookies
Copy link
Author

Yes, it is

@oldalton
Copy link
Member

Great, so it should be fixed by ADAL 2.7.7

@oldalton
Copy link
Member

This should be now fixed in ADAL 2.7.7 release. Please retry and let us know if it's fixed now.

@oldalton oldalton self-assigned this Dec 23, 2018
@nitaeduard
Copy link

nitaeduard commented Jan 4, 2019 via email

@jakkornat
Copy link

Hi. This is not working for versions 4.0.x (recently working with 4.0.3).

ADKeychainTokenCache.defaultKeychain().removeAll(forClientId: ServiceConfig.clientId, error: nil) doesn't clean the token cache.

We should't use the 2.7.x version as it's using UIWebView which is deprecated and will be reject on Apple Review. We also can't use MSAL, bacause we authenticate the internal company users with AD, not ADB2C.

All best.

@oldalton
Copy link
Member

Hi @jakkornat. MSAL also supports AAD flows, just like ADAL.
However, regarding 4.x - are you seeing a UI flash when trying to acquire a token after removeAll is called?

@bondsy777
Copy link

Yes, the problem still exists.

@jakkornat
Copy link

jakkornat commented Oct 1, 2019

@oldalton If you mean that the web view is appearing for a while and then it disappear and the user is logged in again with previous credentials than yes, it does.
I can confirmed that in case of version 2.x works perfect, otherwise the 4.x is not.

@jakkornat
Copy link

@oldalton any update on this?

@oldalton
Copy link
Member

Hi @jakkornat. Sorry for a delay. I think this is because of cookies in the WebView. ADAL 4.x uses WKWebView, and 2.7.x UIWebView. Cookies work differently between those two. ADAL also tries to share datastore between multiple WKWebView instances to maximize SSO.

There're a few things you can try out:

  1. Pass your own webview. That way you can control the cookie state of the webview and pass ADAL a "clean" webview without state.
  2. Pass AD_PROMPT_ALWAYS as a prompt type. That will ensure that regardless of cookies, webview would prompt the user.

Let me know if any of these worked.

@jakkornat
Copy link

@oldalton thanks a lot!
In my case the promptBehaviour parameter was the issue solver!

Have just checked also with a clean WKWebView instance, which also works if we want to manage this by ourselves.

Thanks a lot!

Note: Tested at ADAL 4.0.3, Xcode 11, iOS 13 / iPadOS 13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants