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

Prevent the use of SFAuthenticationSession? #182

Closed
borgeser opened this issue Nov 21, 2017 · 7 comments
Closed

Prevent the use of SFAuthenticationSession? #182

borgeser opened this issue Nov 21, 2017 · 7 comments
Labels

Comments

@borgeser
Copy link

Hi,

I'm creating an app with a lot of webviews (I use SFSafariViewController). I need to share the cookies between the authentication controller provided by the app and my instances of SFSafariViewController.

Everything works well on iOS9 and iOS10 but in iOS11 the library is using SFAuthenticationSession. I understand the purpose of this class but this class forbids the sharing of cookies in my app.

I'd like to know if there is a way to prevent using SFAuthenticationSession in the library. Or maybe if there is another way to achieve my wish: sharing cookies between the authentication process and my SFSafariViewControllers.

Thanks.

@WilliamDenniss
Copy link
Member

Hey,

In your case, I agree that your best bet will be to use SFSafariViewController as you suggest. You'll lose SSO with Safari, but gain it within your app.

AppAuth doesn't currently support this directly, as we assumed everyone wants SSO with Safari. It's trivial to hack the code, you can just delete these lines:

if (@available(iOS 11.0, *)) {
NSString *redirectScheme = request.redirectURL.scheme;
SFAuthenticationSession* authenticationVC =
[[SFAuthenticationSession alloc] initWithURL:requestURL
callbackURLScheme:redirectScheme
completionHandler:^(NSURL * _Nullable callbackURL,
NSError * _Nullable error) {
_authenticationVC = nil;
if (callbackURL) {
[_session resumeAuthorizationFlowWithURL:callbackURL];
} else {
NSError *safariError =
[OIDErrorUtilities errorWithCode:OIDErrorCodeUserCanceledAuthorizationFlow
underlyingError:error
description:nil];
[_session failAuthorizationFlowWithError:safariError];
}
}];
_authenticationVC = authenticationVC;
openedSafari = [authenticationVC start];

Here's what I'd recommend: clone AppAuth into your project, e.g. ./ThirdParty/AppAuth-iOS/. Delete the code I highlighted above. If you're using Pods, simply add pod 'AppAuth', :path => 'ThirdParty/AppAuth-iOS/'. That way the pod is included and built from your local fork.

We could add support to AppAuth, but first I would want to validate that a lot of people have this same use-case. Otherwise, I think it's pretty easy for you to customize how you like as a once-off.

What do you think?

William

@hudeldudel
Copy link

hudeldudel commented Dec 1, 2017

I would also love to see an option to prevent the use of SFAuthenticationSession.
In our case, app stakeholders don't like the system message for our 1st party apps. They accept to loose shared state over having the SFAuthenticationSession privacy message.

@borgeser
Copy link
Author

borgeser commented Dec 4, 2017

Hi William and sorry for the delay,

That is what I've done first, except I didn't want to fork the library so I've just created in my project a custom implementation of OIDAuthorizationUICoordinator. @hudeldudel I recommend you to do that.

But finally SafariViewController wasn't enough because I have also WKWebiews in my app. So I've created another implementation of OIDAuthorizationUICoordinator which only uses WKWebviews.
My work was rather similar to these commits:
equinux@13d034c
equinux@0c55315

Thanks for your help.

@julienbodet
Copy link
Collaborator

I also need to prevent the use of SFAuthentificationSession. Once logged in I want to be able to open a SFSafariViewController using the session cookies from the login. SFSafariViewController shares its cookies between multiple instances in the app.

@WilliamDenniss
Copy link
Member

Good call @borgeser, a custom OIDAuthorizationUICoordinator is definitely what you want to do.

What I should have recommended rather than forking was to make a copy of the OIDAuthorizationUICoordinatorIOS.h/.m class, and then tweak it as you need. #200 has a code sample of doing an authorization with a non-default UI coordinator.

@julienbodet you can use this technique – just create your own OIDAuthorizationUICoordinator, optionally using one of the existing implementations as a starting point.

@ANGOmarcello
Copy link

I need this option as well. To add my opinion on this:
Monkey Patching is always a bad idea, because you lose the ability to update and move fast. So if this would be an included option it would be very great.

@bavarskis
Copy link

I would love this too. I forked the AppAuth and removed the unnecessary lines, but that spells trouble, I'd be much better just to have it out of the box.

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