[GH-ISSUE #223] Both success and failure called after canceling OAuth flow and retrying it

kerem commented

2026-03-03 16:45:53 +03:00

Owner

Originally created by @vronin on GitHub (Apr 14, 2016).
Original GitHub issue: https://github.com/OAuthSwift/OAuthSwift/issues/223

There is a bug in OAuth flow for a following case.

iOS + SFSafariViewController + code grant (however, I think something like that will happen in wide range of scenarios which I haven’t thought through)

The best way to reproduce is to start from empty Simulator (to make sure that no cookies are cached in SFSafariViewController)

Start OAuth flow
It will show SFSafariViewController
Dismiss it without entering credentials (Usually there is a "Done" button)
Start OAuth flow again
It will show SFSafariViewController again
Enter correct credentials
Both authorizeWithCallbackURL success and failure callbacks will be called.

Summary

The root cause is that we register multiple observers (one at step #1 and one at step #4). The observer from step #1 was never removed, because there is no code which handles canceling OAuth flow at step #3.

Details

Taking into account that old observer isn’t removed then on OAuth this function will be called. func application(app: UIApplication, openURL url: NSURL, options: [String : AnyObject]) -> Bool. This function will post notification which will be handled by two observers. One of them will go and get a token and will get 401 (because code is used already).

There are many underlying problems which produce this result

A) The demo application and example show in README instantiate new OAuth2Swift instance for each OAuth flow.

B) OAuthSwift.removeCallbackNotificationObserver called internally only for some results (successful or failed OAuth attempt). However, it won't be called, if a SFSafariViewController is just dismissed and no URL was opened.

I tried to resolved #B (remove old observer) by explicitly calling oauthswift.removeCallbackNotificationObserver(). However, because of #A, the new oauthswift instance doesn't hold a reference to old observer (self.observer will be empty).

Pretty much the only way is to solve this (without modifying OAuthSwift pod code) is to make oauthswift a singleton and call OAuthSwift.removeCallbackNotificationObserver explicitly . However, this opens next problem (also related to observers)

C) If you make OAuthSwift a singleton then it will start crashing at SafariURLHandler.handle at line if let observer = self.observers[key]. The issue is that old instance of SafariURLHandler is not references by anything (because we have to create new authorize_url_handle for each new OAuth flow: oauthswift.authorize_url_handler = SafariURLHandler(viewController: viewController)). As result, self is unowned and not references and a closure is still called ( because observer was never removed ).

I found a temporary workaround/hack for this. The full solution (with several workarounds) are below.

Long term fix

However, I think the logic around registering and removing observers should be rethought to eliminate these workarounds.

Workaround

class OAuthSwiftSingleton {
    static let sharedInstance = OAuthSwiftSingleton()

    let oauthswift : OAuth2Swift!

    init() {
        oauthswift = OAuth2Swift(
            consumerKey:    KEY,
            consumerSecret: SECRET,
            authorizeUrl:   AUTHORIZE_URL,
            accessTokenUrl: AUTHORIZE_TOKEN,
            responseType:   RESPONSE_TYPE
        )
    }
}

...

func startOAuthFlow() {
        let oauthswift = OAuthSwiftSingleton.sharedInstance.oauthswift
        // Check if we have created SafariURLHandler before
        if (oauthswift.authorize_url_handler.dynamicType == SafariURLHandler.self) {
            let safariURLHandler = oauthswift.authorize_url_handler as! SafariURLHandler
            // Get observers out of it
            let observers = safariURLHandler.valueForKey("observers") as! [String: AnyObject]
            // Remove observers from SafariURLHandler
            for a in observers.values {
                NSNotificationCenter.defaultCenter().removeObserver(a)
            }
        }
        oauthswift.authorize_url_handler = SafariURLHandler(viewController: viewController)
        // Remove observers explicitly from OAuthSwift2 singleton
        oauthswift.removeCallbackNotificationObserver()

       oauthswift.authorizeWithCallbackURL( … )
}

Originally created by @vronin on GitHub (Apr 14, 2016). Original GitHub issue: https://github.com/OAuthSwift/OAuthSwift/issues/223 There is a bug in OAuth flow for a following case. iOS + `SFSafariViewController` + code grant (however, I think something like that will happen in wide range of scenarios which I haven’t thought through) The best way to reproduce is to start from empty Simulator (to make sure that no cookies are cached in `SFSafariViewController`) 1) Start OAuth flow 2) It will show `SFSafariViewController` 3) Dismiss it without entering credentials (Usually there is a "Done" button) 4) Start OAuth flow again 5) It will show `SFSafariViewController` again 6) Enter correct credentials 7) Both `authorizeWithCallbackURL` **success** and **failure** callbacks will be called. **Summary** The root cause is that we register multiple observers (one at step #1 and one at step #4). The observer from step #1 was never removed, because there is no code which handles canceling OAuth flow at step #3. **Details** Taking into account that old observer isn’t removed then on OAuth this function will be called. `func application(app: UIApplication, openURL url: NSURL, options: [String : AnyObject]) -> Bool`. This function will post notification which will be handled by two observers. One of them will go and get a token and will get 401 (because code is used already). There are many underlying problems which produce this result A) The demo application and example show in README instantiate new OAuth2Swift instance for each OAuth flow. B) `OAuthSwift.removeCallbackNotificationObserver` called internally only for some results (successful or failed OAuth attempt). However, it won't be called, if a SFSafariViewController is just dismissed and no URL was opened. I tried to resolved #B (remove old observer) by explicitly calling `oauthswift.removeCallbackNotificationObserver()`. However, because of #A, the new oauthswift instance doesn't hold a reference to old observer (`self.observer` will be empty). Pretty much the only way is to solve this (without modifying OAuthSwift pod code) is to make oauthswift a singleton and call `OAuthSwift.removeCallbackNotificationObserver` explicitly . However, this opens next problem (also related to observers) C) If you make OAuthSwift a singleton then it will start crashing at `SafariURLHandler.handle` at line `if let observer = self.observers[key]`. The issue is that old instance of SafariURLHandler is not references by anything (because we have to create new authorize_url_handle for each new OAuth flow: `oauthswift.authorize_url_handler = SafariURLHandler(viewController: viewController)`). As result, `self` is unowned and not references and a closure is still called ( because observer was never removed ). I found a temporary workaround/hack for this. The full solution (with several workarounds) are below. **Long term fix** However, I think the logic around registering and removing observers should be rethought to eliminate these workarounds. **Workaround** ``` class OAuthSwiftSingleton { static let sharedInstance = OAuthSwiftSingleton() let oauthswift : OAuth2Swift! init() { oauthswift = OAuth2Swift( consumerKey: KEY, consumerSecret: SECRET, authorizeUrl: AUTHORIZE_URL, accessTokenUrl: AUTHORIZE_TOKEN, responseType: RESPONSE_TYPE ) } } ... func startOAuthFlow() { let oauthswift = OAuthSwiftSingleton.sharedInstance.oauthswift // Check if we have created SafariURLHandler before if (oauthswift.authorize_url_handler.dynamicType == SafariURLHandler.self) { let safariURLHandler = oauthswift.authorize_url_handler as! SafariURLHandler // Get observers out of it let observers = safariURLHandler.valueForKey("observers") as! [String: AnyObject] // Remove observers from SafariURLHandler for a in observers.values { NSNotificationCenter.defaultCenter().removeObserver(a) } } oauthswift.authorize_url_handler = SafariURLHandler(viewController: viewController) // Remove observers explicitly from OAuthSwift2 singleton oauthswift.removeCallbackNotificationObserver() oauthswift.authorizeWithCallbackURL( … ) } ``` ``` ```

kerem

2026-03-03 16:45:53 +03:00

closed this issue
added the
bug
label

kerem commented

2026-03-03 16:45:54 +03:00

Author

Owner

@phimage commented on GitHub (May 11, 2016):

maybe into 'safariViewControllerDidFinish' we must clear all the observers

observers into dictionary are easy to remove
but the global one, we must put a reference on OAuthSwift object into the SafariURLHandler (or a block to call removeCallbackNotificationObserver)
maybe into the didSet of url handler, by casting on SafariURLHandler or specific protocol

@phimage commented on GitHub (May 11, 2016): maybe into 'safariViewControllerDidFinish' we must clear all the observers observers into dictionary are easy to remove but the global one, we must put a reference on OAuthSwift object into the SafariURLHandler (or a block to call removeCallbackNotificationObserver) maybe into the didSet of url handler, by casting on SafariURLHandler or specific protocol

kerem commented

2026-03-03 16:45:54 +03:00

Author

Owner

@martinpilch commented on GitHub (Jun 30, 2016):

Hi, any progress on this issue? I'm facing same problem

@martinpilch commented on GitHub (Jun 30, 2016): Hi, any progress on this issue? I'm facing same problem

kerem commented

2026-03-03 16:45:55 +03:00

Author

Owner

@phimage commented on GitHub (Jul 25, 2016):

documented function cancel will be added to OAuthSwift (this function call removeCallbackNotificationObserver), function to call when a web view is dismissed without authentification
add a reference to OAuthSwift object to SafariURLHandler`, this allow when pressing "Done" to remove all observers -> library update will make compile issue, but I will document in release...

a current workaround is to let user do the oauthSwift.removeCallbackNotificationObserver() in SFSafariViewControllerDelegate.safariViewControllerDidFinish function
and do also the workaround code from previous comment

let observers = safariURLHandler.valueForKey("observers") as! [String: AnyObject]
            // Remove observers from SafariURLHandler
            for a in observers.values {
                NSNotificationCenter.defaultCenter().removeObserver(a)
            }

@phimage commented on GitHub (Jul 25, 2016): - documented function `cancel` will be added to OAuthSwift (this function call `removeCallbackNotificationObserver`), function to call when a web view is dismissed without authentification - add a reference to `OAuthSwift` object to SafariURLHandler`, this allow when pressing "Done" to remove all observers -> library update will make compile issue, but I will document in release... a current workaround is to let user do the `oauthSwift.removeCallbackNotificationObserver()` in `SFSafariViewControllerDelegate.safariViewControllerDidFinish` function and do also the workaround code from previous comment ``` swift let observers = safariURLHandler.valueForKey("observers") as! [String: AnyObject] // Remove observers from SafariURLHandler for a in observers.values { NSNotificationCenter.defaultCenter().removeObserver(a) } ```

Rows
Columns

[GH-ISSUE #223] Both success and failure called after canceling OAuth flow and retrying it #126