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

Refresh token issues for oAuth2 #42

Closed
arbyruns opened this issue Aug 9, 2022 · 18 comments · Fixed by #43
Closed

Refresh token issues for oAuth2 #42

arbyruns opened this issue Aug 9, 2022 · 18 comments · Fixed by #43

Comments

@arbyruns
Copy link

arbyruns commented Aug 9, 2022

I've been running into an issue with refresh tokens. I have no issues logging in for the user, but I'm finding if I come back several hours later I'm unauthorized and have to initiate the login process all over again.

This is what I'm running at start of the update in my AppDelegate. Do I need to refreshOAuth2AccessToken at initiateLogin?

func initiateLogin(forceReconnect: Bool = false, completion: @escaping (Bool)->Void) {
     Task {
         try await container.client?.refreshOAuth2AccessToken() // I don't know if this will do anything, but I'm trying.
         await loginUser(forceReconnect: forceReconnect, completion: { user in

             if let encoded = try? self.encoder.encode(user) {
                 self.defaults.set(encoded, forKey: ConstantStrings.authKeyChainString)
                 self.initClient(withCredentials: user)
                 completion(true)
             }
       })
     }
   }


     func loginUser(forceReconnect: Bool = false, completion: @escaping (OAuth2User)->Void) async {
     do {
       // flow with existing user
         if let savedUser = defaults.object(forKey: ConstantStrings.authKeyChainString) as? Data {
             if let oauthUser = try? decoder.decode(OAuth2User.self, from: savedUser), !forceReconnect {
                 completion(oauthUser)
             } else {
                 // flow with new user
                 let oauthUser = try await Twift.Authentication().authenticateUser(
                   clientId: TWITTER_clientId,
                   redirectUri: URL(string: TWITTER_CALL_BACKURL)!,
                   scope: Set(OAuth2Scope.allCases),
                   presentationContextProvider: nil)
                 completion(oauthUser)
               }
         }
     } catch {
       print(error.localizedDescription)
     }
   }


   func initClient(withCredentials oauthUser: OAuth2User) {
       container.client = Twift(oauth2User: oauthUser, onTokenRefresh: { newUser in
           if let encoded = try? self.encoder.encode(newUser) {
               self.defaults.set(encoded, forKey: ConstantStrings.authKeyChainString)
           }
       })
   }
@emin-grbo
Copy link
Contributor

Ok so since I had some issues with this as well. Important question first....are you using on the simulator or the phone?
But more importantly, are you switching back and forth?

Secondly, once you start the app after few hours, have you tracked where the code ends up? Meaning if it gets the save credentials and send them and if it does, will it go for the refresh etc...
Try to put some breakpoints in there so we can see where you end up.

Looking at it right here, it kinda seems like it should work. You save the token again on refresh so it should be 👍

@arbyruns
Copy link
Author

arbyruns commented Aug 9, 2022

I've been using both and to be honest I just added try await container.client?.refreshOAuth2AccessToken() to my code and have not tried this new code on my device. So if I understand correctly refreshOAuth2AccessToken() will get a new token regardless of status?

I'm going to try with the new code on an actual device.

@damartin1
Copy link

I am using this on my actual device and I made sure to include .offlineaccess to the my scope and it still doesn't refresh the token. Been working on this for a couple of days.....

@emin-grbo
Copy link
Contributor

To be safe, make sure you are using it on one device while testing, as changing it will ask you to replace the token or re-log and I believe you can be logged in with one device via one app.

try await container.client?.refreshOAuth2AccessToken() will refresh the token, yes, but you are calling it before the client is initialized, so I would guess it just fails as client is nil.
Screenshot 2022-08-09 at 20 17 14

I also think that the issue might be in the container you are using here?

try to follow the breakpoints and see where you land in this flow. We need to see if the user is saved and if so, did it only safe the old one?

@emin-grbo
Copy link
Contributor

Where is the Initiate login called? Try using forceRefresh as well?

Have either of you tried the demo app @daneden posted in the repo by any chance?

@arbyruns
Copy link
Author

arbyruns commented Aug 9, 2022

Thanks! I'm calling initiateLogin in AppDelegate didFinishLaunchingWithOptions. I did not use the demo app as I could not figure out the .env configuration.

The container i'm using is:

class ClientContainer: ObservableObject {
    @Published var client: Twift?
    @Published var launchErrorAlert = false
    @Published var launchErrorMessage = ""
}

From what I can tell in my breakpoints the new user is getting updated, but will double check.

@emin-grbo
Copy link
Contributor

I see. At least client does not need to be Published property, response data can be, as in @Published var tweets... for example.

And you are forwarding that client to the main view or how is it accessed? I guess it is initialised then in the AppDelegate as @StateObject?

@arbyruns
Copy link
Author

arbyruns commented Aug 9, 2022

I have it as @StateObject var container = ClientContainer() in @Main of my app and then I have the following

var body: some Scene {
        WindowGroup {
            ZStack {
                if let twitterClient = container.client {
                    ContentView()
                        .environmentObject(twitterClient)
                        ....

I set up some breakpoints and it seems that after two hours I'm getting Unauthorized so maybe my refresh token is in the wrong spot like you stated above?

@emin-grbo
Copy link
Contributor

Could be yeah. I see why client is @published now, so main view can refresh when you initialize it i guess?

So for the refresh, try sending the forceRefresh to see how it behaves after 2h, but really drawing a blank as to why this is failing to get the user. Does it ever try to decode the saved user, and is there anything saved ever?

@arbyruns
Copy link
Author

arbyruns commented Aug 9, 2022

I probably sound like I have no idea what I'm doing :D, but I have this function below that calls refreshOAuth2AccessToken() during scene change.

func getRefreshToken() {
     Task {
         print(#function)
         if let savedUser = defaults.object(forKey: ConstantStrings.authKeyChainString) as? Data {

             if let loadedUser = try? decoder.decode(OAuth2User.self, from: savedUser) {
                 print("loadedUser: \(loadedUser)")

                 container.client = Twift(oauth2User: loadedUser, onTokenRefresh: { newUser in
                     if let encoded = try? encoder.encode(newUser) {
                         defaults.set(encoded, forKey: ConstantStrings.authKeyChainString)
                     }
                 })

                 try? await container.client?.refreshOAuth2AccessToken()

                 withAnimation {
                     showLoginScreen = false
                 }
             }

         }
     }
 }

SceneChange

    var body: some Scene {
        WindowGroup {
            ZStack {
                if let twitterClient = container.client {
                    ContentView()
                        .environmentObject(twitterClient)
                } else {
                    Text("Logging In")
                    LoginView()
                        .edgesIgnoringSafeArea(.all)
                }
            }
            .alert(isPresented: $showErrorAlert) {
                Alert(title: Text("Failed to get token"), message: Text(errorAlertMessage),
                      dismissButton: .default(Text("Ok")))
            }
        }
        .onChange(of: scenePhase) { newPhase in
            if newPhase == .active {
                print("Active")
            } else if newPhase == .inactive {
                print("Inactive")
            } else if newPhase == .background {
                print("Background")
                getRefreshToken()
            }
        }
    }

Let me set some breakpoints and check back to see what happens. I'll need to wait for the two~ish hour window.

@arbyruns
Copy link
Author

arbyruns commented Aug 9, 2022

Ok I set up some break points and let it fail. I called getRefreshToken() listed above and I get back the following error UnknownError(Optional("Couldn\'t find refresh token or client ID")).

I can see that my refresh token is still good, but the clientID is nil.

OAuth2User(clientId: nil, accessToken: 
"Removed...jAwNzM5Mzc0NjQ6MTowOmF0OjE", refreshToken: 
Optional("Removed...jAwNzM5Mzc0NjQ6MToxOnJ0OjE"), expiresAt: 2022-08-10 00:38:03 +0000, scope: Set([Twift.OAuth2Scope.followsRead, 
Twift.OAuth2Scope.tweetWrite, Twift.OAuth2Scope.blockRead, Twift.OAuth2Scope.listRead, 
Twift.OAuth2Scope.bookmarkRead, Twift.OAuth2Scope.muteWrite, Twift.OAuth2Scope.tweetModerateWrite, 
Twift.OAuth2Scope.likeWrite, Twift.OAuth2Scope.offlineAccess, Twift.OAuth2Scope.spaceRead, 
Twift.OAuth2Scope.followsWrite, Twift.OAuth2Scope.muteRead, Twift.OAuth2Scope.usersRead, 
Twift.OAuth2Scope.blockWrite, Twift.OAuth2Scope.bookmarkWrite, Twift.OAuth2Scope.tweetRead, 
Twift.OAuth2Scope.listWrite, Twift.OAuth2Scope.likeRead]))

@daneden
Copy link
Owner

daneden commented Aug 10, 2022

Hey guys! Thanks for reporting this issue and providing so much detail @arbyruns. I’m doing some local testing myself now and have run into at least a couple of potential root issues which I’ll fix and elaborate on. For one, I didn't write the encoder properly for OAuth2User structs, so token expiration dates aren't correctly stored and retrieved. This will be fixed soon, but I will ensure this original issue is fixed before pushing any code.

I’m also going to update the test app to demonstrate how to securely store tokens for easier reference too (should help with #40).

@arbyruns
Copy link
Author

@daneden thank you so much! Let me know if there's anything else I can help with.

@arbyruns
Copy link
Author

arbyruns commented Aug 12, 2022

@roblack what you provided works great on launch of the app, but how can I go about implementing your workaround within a button? I call initiateLogin, but nothing happens. Any thoughts?

     twiftLoginViewModel.initiateLogin { value in
                                    print("value")
                                }

@daneden
Copy link
Owner

daneden commented Aug 12, 2022

Hey @arbyruns, you should try updating to the new release of Twift (v0.2.1) and see if it resolves your issue. As described in #43, there was an issue with how refresh tokens were encoded for storage, which may have been contributing to the problem you encountered.

It's worth bearing in mind that you should not have to manually refresh the authentication token; all of Twift's network calls check the token expiration and attempt to refresh it if necessary. You can (for debugging reasons, for example) force a refresh using the refreshOAuth2AccessToken(onlyIfExpired: false) method.

@arbyruns
Copy link
Author

Thanks I'll give it a try! Just for my own clarity, I don't need to call client?.refreshOAuth2AccessToken()

@daneden
Copy link
Owner

daneden commented Aug 12, 2022

@arbyruns correct! Every network call in Twift is eventually handled by the internal call method (

internal func call<T: Codable>(route: APIRoute,
), which in turn will invoke the refreshOAuth2AccessToken(onlyIfExpired: true) function. This ensures that tokens are refreshed on your behalf only when necessary, but you still have the option to manually refresh them in order to handle the new token (e.g. if you want to update stored credentials for some reason)

@arbyruns
Copy link
Author

Thank you for the explanation and for the quick turn around!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants