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

Custom scheme redirect iOS 11 #232

Open
MarcoTSS2018 opened this issue Apr 25, 2018 · 19 comments
Open

Custom scheme redirect iOS 11 #232

MarcoTSS2018 opened this issue Apr 25, 2018 · 19 comments
Labels
provider-bug Issues that are affected by bugs or spec non-compliance by an OAuth2 AS / OIDC IDP

Comments

@MarcoTSS2018
Copy link

Hi @ALL
We are facing the following problem. Our authentication service only accepts a https scheme for the redirect uri. So it is not possible to use a custom scheme for a deep linking into to app. So we have implemented a redirect page with a button, which references to a custom scheme like com.myapp://token=abcd1234. This solution works perfectly for iOS 10 and below. On iOS 11 where SFAuthenticationSession is used the library closes the web view without calling any callback. The library seems to block the callbacks when the redirect uri scheme doesn't match the custom scheme.

Is there a way to omit this behavior?

@bbqsrc
Copy link

bbqsrc commented May 7, 2018

I can confirm this occurs due to this function in OIDAuthorizationService.m:

- (BOOL)shouldHandleURL:(NSURL *)URL {
  NSURL *standardizedURL = [URL standardizedURL];
  NSURL *standardizedRedirectURL = [_request.redirectURL standardizedURL];

  return OIDIsEqualIncludingNil(standardizedURL.scheme, standardizedRedirectURL.scheme) &&
      OIDIsEqualIncludingNil(standardizedURL.user, standardizedRedirectURL.user) &&
      OIDIsEqualIncludingNil(standardizedURL.password, standardizedRedirectURL.password) &&
      OIDIsEqualIncludingNil(standardizedURL.host, standardizedRedirectURL.host) &&
      OIDIsEqualIncludingNil(standardizedURL.port, standardizedRedirectURL.port) &&
      OIDIsEqualIncludingNil(standardizedURL.path, standardizedRedirectURL.path);
}

When I replace it with return YES; my custom URI scheme works as expected.

@bbqsrc
Copy link

bbqsrc commented May 7, 2018

A fix is available on my fork.

@StevenEWright
Copy link
Collaborator

@bbqsrc Any reason not to create a pull request? Also, I would be interested to hear more about the assumption that was being made and why that didn’t work for you.

@bbqsrc
Copy link

bbqsrc commented May 8, 2018

I haven't tested it thoroughly yet nor decided whether it is the best course of action. Once I've done that I'll provide a pull request, as I've done on the Android version for similar issues. 😄

The assumption made was that the common pattern of using a real redirect URI that has a location header redirecting to a custom scheme (eg com.example.mycallback) has no way of being specified. The function compares the URLs 1:1. By adding the chunk of code that I added, it checks the Info.plist for the supported custom schemes list and checks against that before assuming that the redirectURL can be trusted.

@StevenEWright
Copy link
Collaborator

One concern I have is that integrators will place their call to resumeExternalUserAgentFlowWithURL: in their app delegate’s handleURL:... method, and their application will crash with the OIDOAuthExceptionInvalidAuthorizationFlow exception thrown in that method if they get launched with one of their custom url schemes for any other reason.

@WilliamDenniss assuming we come up with a reliable approach to determine whether the request is for us or not, do you have any other concerns?

@bbqsrc
Copy link

bbqsrc commented May 8, 2018

I don't understand why this is something that the library should handle and not something that the implementer should handle. Adding a check for the correct scheme and expected URL path in the app delegate is not onerous and keeps the API clean.

You could add a convenience function for doing this too, as is already kind of there.

My current implementation is like this anyway:

    func application(_ app: UIApplication, open url: URL, options: [UIApplicationOpenURLOptionsKey : Any] = [:]) -> Bool {
        switch url.scheme {
        case "com.example.thingo":
            let url = ThingoOAuth.instance.redirectURL(fromCustom: url)
            ThingoOAuth.currentAuthFlow?.resumeAuthorizationFlow(with: url)
            return true
        default:
            break
        }
        
        return false
    }

@StevenEWright
Copy link
Collaborator

StevenEWright commented May 8, 2018

Generally it's pretty clear when AppAuth is supposed to handle a URL.

The only reason it's not in this case is because the IdP doesn't support custom schemes, and the implementor is going outside the typical flow to make it work.

As a general principle I'd say the common thing should be easy, and the workaround should be possible.

Personally, I feel the current implementation makes sense for most people in most situations, and would advocate for leaving it as the default behavior. That said, I certainly think we should make this flow possible as well.

Why not a simple refactor like:

- (BOOL)resumeExternalUserAgentFlowWithURL:(NSURL *)URL {
  // rejects URLs that don't match redirect (these may be completely unrelated to the authorization)
  if (![self shouldHandleURL:URL]) {
    return NO;
  }
  return [self uncheckedResumeExternalUserAgentFlowWithURL:URL];
}

- (BOOL)uncheckedResumeExternalUserAgentFlowWithURL:(NSURL *)URL {
  // checks for an invalid state
  ...

... which would allow implementors in cases like this one to call uncheckedResumeExternalUserAgentFlowWithURL: explicitly if they care to do so?

Edited to rename method suggestion from unsafe to unchecked, which seems like a slightly clearer suggestion.

@bbqsrc
Copy link

bbqsrc commented May 8, 2018

I don't know what IdP stands for, but having a custom scheme is something I've always encountered. I've never used a platform or worked for a company that hasn't used a custom scheme where it is available as it solves so many edge cases and oddities across OS versions and platforms.

On Android, the AppAuth implementation supports handling both as a matter of course in the manifest XML itself, so this is even a feature within your own pool of AppAuth implementations.

Perhaps the easiest option is to add an optional parameter called extraCallbackURLs on the authorization and token request objects, which is an array of URLs that a callback may be handled on. That was there's no surprises, it's explicit and handles the edge cases without requiring a subclass.

@StevenEWright
Copy link
Collaborator

IdP just stands for "Identity Provider". The OP referred to theirs as an "authentication service"... same thing.

They also said:

"[Their IdP] only accepts a https scheme for the redirect uri."

... and does not support custom schemes. I thought this was the edge case we were trying to solve for here?

On Android, the AppAuth implementation supports handling both as a matter of course in the manifest XML

Perhaps the easiest option is to add an optional parameter called extraCallbackURLs on the authorization and token request objects

So, just to recap, it sounds like we are discussing three possible options for a solution?

  1. Let AppAuth clients write their own logic in their UIApplicationDelegate handleURL:... method.
  2. Support some settings in an info.plist, analagous to Android.
  3. Support some settings passed to "authorization and token request objects" at runtime.

I only based my suggestion (which is a way of supporting solution 1) on what I thought was your reasonable opinion that:

Adding a check for the correct scheme and expected URL path in the app delegate is not onerous and keeps the API clean.

The other options you've mentioned sound completely reasonable as well - though I'd think supporting solutions 2 and 3 would definitely be more work for AppAuth iOS than supporting solution 1 (which is basically a three line change, plus documentation).

Having some parity with Android is certainly a good selling point for solution 2.

Do you have any thoughts on the relative merits of these three solutions? I don't have a lot of experience with the AppAuth Android implementation, so I'd love to hear what you think especially about solution 1 vs. solution 2.

@bbqsrc
Copy link

bbqsrc commented May 8, 2018

Thanks for the jargon glossary. :)

Yes, this is the case we are solving: that the provider does not allow for custom redirect URLs, and requires http or https, and having that redirect URL do a redirect of its own to a custom scheme.

Solution 2 is by far the easiest to solve this problem, and analogous to what Android does. I could provide a proof of concept that links the "real" redirect URL to an array of other possibilities to handled if you would like? It would not be different to my current fork beyond looking at an OIDRedirectURLs dictionary or something similarly named to get a well-known list of possibilities.

@StevenEWright
Copy link
Collaborator

@WilliamDenniss @iainmcgin @tikurahul Opinions?

@vladikoff
Copy link

vladikoff commented May 9, 2018 via email

@StevenEWright
Copy link
Collaborator

@vladikoff Thank you for weighing in! Much appreciate the cross platform insight.

@iainmcgin
Copy link
Member

I think it would be useful to allow passing a set of accepted redirect URIs for a request that canHandleURL matches against (option 3?). This set may or may be disjoint from the specified redirect URI, to accommodate this case where the redirect is captured and rewritten by an intermediary.

Generally speaking, I think that it's OK for this to look different from the way Android configures redirect URIs, because the two platforms have rather different approaches for routing URIs to handlers.

@bbqsrc did you try using a universal link registration to capture the https redirect? If that works for your use case, it would likely be the best solution and require no immediate changes from AppAuth.

https://developer.apple.com/ios/universal-links/

@bbqsrc
Copy link

bbqsrc commented May 14, 2018

Universal links require control over the server side and configuring a .well-known endpoint for ensuring it works. Even when I configured this for a previous client, I could never consistently get this to work on all devices, so a custom schema fallback was always required.

@bbqsrc
Copy link

bbqsrc commented May 14, 2018

@iainmcgin to assist with deciding on an approach here, I am thinking about platform expectations.

It is already expected that custom URI schemes have to be listed in an Info.plist file. It is already an expectation that a redirect URI must be specified to the OAuth client handler. It is also expected that a URI that is registered with the application will call into the AppDelegate for the application to handle, or otherwise is handled by a similar callback as is the case with the embedded browser session.

It seems that based on this, both option 2 (something in the Info.plist) and option 3 (explicitly augment the request objects to support extra possible redirect URIs) are both expected and understandable solutions to the problem from a platform-specific perspective.

The question ultimately comes down to: do we treat an alternative URI as a configuration option or an explicit coding decision? If it's more configuration, option 2, else option 3.

@WilliamDenniss
Copy link
Member

I prefer option 3, as option 2 is basically global configuration which may not always be appropriate. I don't really like that this would bloat the constructor arguments and property count though.

From a purist perspective, we shouldn't need to support this, but from a practical standpoint I can see why people want it.

I do worry that these authorization servers that only support https redirects, may also assume confidentiality of the clients, which is a concern then if those clients are used in native apps where only public clients should be used. The ideal (but complicated) approach is to chain the OAuth in these cases, where the app does a full OAuth flow to it's own server (using AppAuth), which then has an embedded OAuth flow to the IdP.

@lapinek
Copy link

lapinek commented Jun 3, 2019

Universal links require control over the server side and configuring a .well-known endpoint for ensuring it works. Even when I configured this for a previous client, I could never consistently get this to work on all devices, so a custom schema fallback was always required.

@bbqsrc, could you please provide more details on what the issues were and whether/how they were related to a device used?

From a purist perspective, we shouldn't need to support this, but from a practical standpoint I can see why people want it.

I do worry that these authorization servers that only support https redirects, may also assume confidentiality of the clients, which is a concern then if those clients are used in native apps where only public clients should be used. The ideal (but complicated) approach is to chain the OAuth in these cases, where the app does a full OAuth flow to it's own server (using AppAuth), which then has an embedded OAuth flow to the IdP.

This sounds right, although the spec requires explicit registration of such client as a public one. Still, the upstream library should probably not promote circumventing the authorization server requirement for identifiable redirection URI. It should be up to the authorization server to fully comply with the native apps support recommendations.

On a separate note, if an extra screen is not an issue (#396), Universal Links may be the most elegant solution in iOS 11-12 when "https" redirection is required (as @iainmcgin suggested).

ksuther added a commit to flexibits/AppAuth-iOS that referenced this issue Jan 15, 2020
… match the redirect URL sent to the server

Handle the case where _request.state is nil but response.state is @""

openid#232
bbqsrc@f7a37d3
ksuther added a commit to flexibits/AppAuth-iOS that referenced this issue Jul 23, 2021
… match the redirect URL sent to the server

Handle the case where _request.state is nil but response.state is @""

openid#232
bbqsrc@f7a37d3
@pavelsg
Copy link

pavelsg commented Jun 14, 2023

That's surprising how little attention this gets, considering https://developer.apple.com/app-store/review/guidelines/#sign-in-with-apple which states that it's necessary to have Sign In With Apple (SIWA) implemented to pass AppStore review if an app implements sign-in with other social platforms (with few exceptions).

I have my App setup for use of Universal Links and it opens content just fine, but AppleID makes HTTP POST request to RedirectURL which doesn't seem to return control to the app. I have worked this around on Android using different RedirectURL (which points to my API) and from there I redirect to either to HTTPS or custom scheme (both works fine on Android) with appropriate query params for AppAuth-Android to take control over and continue the flow.

To sum up:

  • Universal Links doesn't seems to be suitable solution for SIWA on iOS (due to POST requests handling by web-view)
  • Custom schemes are not supported for RedirectURLs on IdP (Apple) side.
  • Workaround with proxying POST request through the API is not possible due to AppAuth-iOS 'shouldHandleURL' logic

Does anyone have other workaround suggestions (other than forking AppAuth-iOS)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
provider-bug Issues that are affected by bugs or spec non-compliance by an OAuth2 AS / OIDC IDP
Projects
None yet
Development

No branches or pull requests

8 participants