AppAuth-iOS icon indicating copy to clipboard operation
AppAuth-iOS copied to clipboard

AuthState callback isn't called if the redirect URI does not end with "/"

Open emma-guilbault-enki opened this issue 1 year ago • 2 comments

Describe the bug I developed an app using OpenID Connect (on the Microsoft identity platform) and here is what I noticed: AuthState callback isn't called when the connection succeeds if the redirect URI doesn't end with "/". However, it is called whenever the connection process is interrupted (for example, if the "Cancel" button is tapped). I finally found a solution when I read this issue comment: https://github.com/openid/AppAuth-iOS/issues/197#issuecomment-597212857. So it seems necessary to add "/" at the end of the redirect URI.

To Reproduce Clone the example project of AppAuth-iOS and configure it with your own issuer, client ID and redirect URI. The redirect URI must not end with a "/". Launch the project, enter your login and password, and validate the connection. Then it correctly redirects to the app, but the callback isn't called.

Expected behavior It should call the callback (either raising an error to report it or succeeding).

Environment

  • Device: iPhone 14 (or any other device running iOS 16)
  • OS: iOS 16

Additional context This bug doesn't exist in the Android library (AppAuth-Android: https://github.com/openid/AppAuth-Android): the callback is always called, even though the redirect URI doesn't end with "/".

emma-guilbault-enki avatar Jun 21 '23 07:06 emma-guilbault-enki

I think this has something to do with how AppAuth-iOS uses [-[NSURL standardizedURL]](https://developer.apple.com/documentation/foundation/nsurl/1411073-standardizedurl?language=objc) in OIDAuthorizationService.

It seems like -[NSURL standardizedURL] doesn't strip out the trailing / and so the equality check in OIDIsEqualIncludingNil fails.

You can verify this with a simple test:

let red1 = "www.foo.com"
let u1 = URL(string: red1)!

let red2 = "www.foo.com/"
let u2 = URL(string: red2)!

let u1Standardized = u1.standardized
let u2Standardized = u2.standardized

u1Standardized == u2Standardized // false

IMHO, this makes the issue more about ensuring that the URLs match correctly, which I believe should be not be done in the library as we would have to special case things like this in some way (not impossible, but just would need to be prioritized).

mdmathias avatar Jun 21 '23 17:06 mdmathias

We've also run into this with Microsoft OpenID. I think there are three related issues I've seen:

  • The trailing / as noted
  • Microsoft also lower-cases the returned URL, so a config with a redirect URL that includes capital letters will also fail the matching mechanism.
  • More fundamentally, if ASWebAuthenticationSession is calling the completion handler, AppAuth should always call it as well, either with an error or returned auth data as appropriate.

robmathers avatar Nov 09 '23 16:11 robmathers