auth0 / Auth0.swift

Auth0 SDK for Apple platforms
https://auth0.github.io/Auth0.swift
MIT License
351 stars 230 forks source link

Client ID and redirect URL not present in clear session URL #187

Closed Rypac closed 6 years ago

Rypac commented 6 years ago

When using the WebAuth.clearSession API to log out of an Auth0 session on iOS 11, the Client ID and redirect URL are not included in the clear session URL used to invoke the SFAuthenticationSession.

This issue is only present if the federated parameter is false.

cocojoe commented 6 years ago

It should attempt to callback to the app, did you add the callback URL to the Allowed Logout URls?

https://github.com/auth0/auth0.swift#upgrade-notes

Personally I feel that the behaviour allowed by SFAuthenticationSession is more intrusive than the behaviour in the SFSafariViewController in iOS 10. Another approach is set a flag and the next time you call WebAuth use prompt=login to force a fresh login.

e.g.

.parameters(["prompt":"login"])
Rypac commented 6 years ago

Thanks for the response @cocojoe.

Yes, the callback URL has been added to the Allowed Logout URLs. I believe I've found the issue as to why the redirect URL is not present in the clear session URL and have addressed it in #188.

In short, the change is:

@@ -211,7 +211,8 @@ class SafariWebAuth: WebAuth {
    let returnTo = URLQueryItem(name: "returnTo", value: self.redirectURL?.absoluteString)
    let clientId = URLQueryItem(name: "client_id", value: self.clientId)
    var components = URLComponents(url: logoutURL, resolvingAgainstBaseURL: true)
-   components?.queryItems?.append(contentsOf: [returnTo, clientId])
+   let queryItems = components?.queryItems ?? []
+   components?.queryItems = queryItems + [returnTo, clientId]
    guard let clearSessionURL = components?.url, let redirectURL = returnTo.value else {
      return callback(false)
    }

When federated is not present in logoutURL the components?.queryItems is nil. Therefore appending the returnTo and clientId query items has no effect and will not be included in clearSessionURL.


I agree with your point about the SFAuthenticationSession being more intrusive. Another option I have been playing around with was removing the session data from the keychain and using the prompt=login query parameter that you mentioned to force a new login.

My only reservation about this approach is the session not being cleared from the Safari website data. Unless I am mistaken, will signing into the same and authenticating with Auth0 not just redirect you to the app using the session saved in Safari?

This would not be ideal if the user has previously logged out.

cocojoe commented 6 years ago

You are right this is a bug, thanks. I'll raise a PR for this.

If you try the prompt=login the IdP should require re-authentication. So in the case of FB it will prompt for the password to be re-entered. You have to go with what works best for your use case.

Rypac commented 6 years ago

Great. I added a fix in PR #188 but feel free to raise your own. I briefly looked at writing some tests but there didn't seem to be a simple way to do it for the SFAuthenticationSession.

cocojoe commented 6 years ago

Merged