xiao99xiao / TwitterLoginKit

A Twitter Login library to replace the deprecated TwitterKit.
MIT License
11 stars 5 forks source link

sourceApplication is nil if iOS 13 #1

Closed jiggaman0412 closed 5 years ago

jiggaman0412 commented 5 years ago

Thank you for releasing this library. I use this library for production application. But I found that this is not working with iOS 13 build with Xcode Beta 6 ver 11.

I temporarily fixed this issue by avoiding guard safe and using this library by fork. So far, this works very well on whether you have Twitter Client App or not.

But I'm not sure if this change will cause some effect on other. Could you revise if you have a spare time.

xiao99xiao commented 5 years ago

@jiggaman0412 I just checked the changes and pushed a commit. You can try and see if it's solved. https://github.com/xiao99xiao/TwitterLoginKit/commit/aa88cbc5203109bfd7949da7267508b7988acea1

jiggaman0412 commented 5 years ago

@xiao99xiao Thanks for quick reply and fix. I just looked through your commits and had one question.

This does not work if you have twitter client application because whether you have twitter app or not you can get valid auth token from twitter and always return true. So that mobileSSO.process(redirectURL: url) never run.

if #available(iOS 13.0, *) {
            if mobileSSO.verifyOauthTokenResponse(fromURL: url) {
                return webAuthenticationFlow?.resumeAuthentication(withRedirectURL: url) ?? false
            } else {
                return mobileSSO.process(redirectURL: url)
            }
        }

One suggestion is that sourceApplication is not nil but contains "com.apple" if twitter is opened on safari.

xiao99xiao commented 5 years ago

@jiggaman0412 Sorry I don't have any iOS 13 devices currently.

Did you mean that on iOS 13, sourceApplication would still be non-nil if the app is opened by Safari, have you test it? I thought only app that belongs to the same developer will show up as sourceApplication on iOS 13, as WWDC video indicates.

jiggaman0412 commented 5 years ago

@xiao99xiao

I thought only apps that belongs to the same developer will show up as sourceApplication on iOS 13, as WWDC video indicates.

This is correct. So you can't get sourceApplication from other developer.

Did you mean that on iOS 13, sourceApplication would still be non-nil if the app is opened by Safari, have you test it?

But you can still get sourceApplication from SafariView which returns me "com.apple.SafariViewService" when you open twitter on Safari. I tested it.

スクリーンショット 2019-09-06 11 51 50

xiao99xiao commented 5 years ago

@jiggaman0412 Thanks for clarifying.

This does not work if you have twitter client application because whether you have twitter app or not you can get valid auth token from twitter and always return true.

I updated the isOauthTokenVerified(from url: URL?) -> Bool method to actually return false when there is no oauth_token or denied key in the url, which does not exist in Twitter app's callback. https://github.com/xiao99xiao/TwitterLoginKit/commit/0833190b4cdb3c9414862a7ed76a59d6d5ea4bba

The Twitter app's callback is like twitterkit-<Consumer Key>://secret=<Secret>&token=<Token>&username=<User Name>

So the library check if the url is a Oauth callback first. If not, then try the mobileSSO process.

But you can still get sourceApplication from SafariView which returns me "com.apple.SafariViewService" when you open twitter on Safari.

I see. However, since we are removing the sourceApplication verification for Twitter app, I'd prefer to remove the whole typeOf(sourceApplication:) thing. Determining the url type from the parameters is enough. https://github.com/xiao99xiao/TwitterLoginKit/commit/46d6ebf3d754083df3a5a3a3b7c06fadf3aa0479

jiggaman0412 commented 5 years ago

@xiao99xiao Cool and quick coding!! Thanks, you did great job for our company.

One last thing is below. This was my part, but I found another warnings, so if you have time to fix, please do.

image

I think all you have to do is just below.

public func performWebBasedLogin(_ viewController: UIViewController, completion: @escaping TwitterLoginCompletion) {
        guard let authConfig = authConfig, let urlParser = urlParser else {return}
        webAuthenticationFlow = TwitterWebAuthenticationFlow(authConfig: authConfig, urlParser: urlParser)
        webAuthenticationFlow?.beginAuthenticationFlow({ (vc) in
            DispatchQueue.main.async {
                viewController.present(vc, animated: true, completion: nil)
            }
        }, completion: { (state) in
            DispatchQueue.main.async {
                if let vc = viewController.presentedViewController as? SFSafariViewController {

                    vc.dismiss(animated: true, completion: {
                        completion(state)
                    })
                } else {
                    completion(state)
                }
            }
        })
    }
xiao99xiao commented 5 years ago

@jiggaman0412 Fixed at https://github.com/xiao99xiao/TwitterLoginKit/commit/a7f41599a78e10db94fbe35a3c4ec0ca86d4bd75 .