mackuba/SafariAutoLoginTest

Suggestion: Mention using iOS 9's universal app links in readme.

rsattar opened this issue · 6 comments

I'm so glad you made this demo version of the autologin with SFSafariViewController. It was on my list to do at some point, but I'm glad you have done it!

I'm always worried about people thinking this is an "unsafe" way to do things. That said, the only place I've found that is "unsafe" is when the webpage is redirecting back to you. It's possible using the regular url schemes that another app could have the same url scheme registered in their info.plist, and that app would open instead (and potentially do malicious things).

This is completely solved by iOS 9's Universal Links, which lets your app register URLs it will handle, and prevent others from hijacking your domain/urls.

More info: https://goo.gl/ucc1th

Glad you like it :) I was wondering how you've done it, especially the hiding of the Safari controller - I tried to not present it at all, but then it doesn't load anything at all... Did you do something like here with the alpha etc.?

To be honest, the main reason I'd think this might be unsafe would be that Apple might decide they don't like it when apps do it this way... Especially since I have a bad feeling that this could be used in some evil ways too. Do you think there is such risk? (Of either getting rejected or them changing the API somehow.)

And yes, I'll need to check if this works with universal links too and update the readme. Although I guess in that case you need to be careful to not include the URL that you want to load in SVC on the site association list, right?

Yup, basically:

self.safariViewController = [[SFSafariViewController alloc] initWithURL:autologinURL];
self.safariViewController.modalPresentationStyle = UIModalPresentationOverFullScreen;
self.safariViewController.view.alpha = 0.0;
[self.window.rootViewController presentViewController:self.safariViewController
                                                                        animated:NO
                                                                     completion:nil];

alpha = 0, animation: NO

Yeah, as far as "security" goes, it's safe (if you're using universal app links).
A malicious app can certainly find out about, and open your special redirecting url. However, unless that app can also be called back by the web page (instead of your app), then the communication is safe. The universal app links in iOS 9 let's you register your own urls so that only your app gets called back by it.

Also, ideally, what the server is sending back to you is NOT the plaintext password or username, but instead some sort of one-time use token that your app can log in with (and then receive the username and presumably oauth token back from the response, which is then saved).

As far as your special URL, you can obfuscate but know that it cannot be kept secret forever. So you can't rely on it unfortunately.

Additionally, it's probably a good idea to ensure that the webpage that does the redirect has some safety checks to ensure it's not running in an iframe, like so:

function inIframe () {
    try {
        return window.self !== window.top;
    } catch (e) {
        return true;
    }
}

(from Stack Overflow)

These don't need to be in your demo, FYI. I'm just pointing out that once we use this in production (and we do plan to use it for Cluster, we need to consider someone actively trying to break this.

FWIW, I've mentioned universal app links in the readme as a possible alternative, though I haven't tried it, and the whole old approach doesn't really work anymore. I've added an alternative flow using SFAuthenticationSession, but I don't know if that's compatible with universal links either.

Looks like SFAuthenticationSession doesn't support universal links, only custom schemes: https://twitter.com/rmondello/status/884521414929395712