nxtbgthng / OAuth2Client

Client library for OAuth2 (currently built against draft 10 of the OAuth2 spec)
855 stars 217 forks source link

sporadic logout / loss of token issue #178

Open cowboyrushforth opened 9 years ago

cowboyrushforth commented 9 years ago

Hi,

Thanks for the great library. I have been using it in my app, which relies on accessing an OAuth2 api which has a server side library for ruby on rails called doorkeeper. Overall the integration works great.

In recent weeks, roughly 10% of my install base have been having sporadic cases of what appear to be needing to re-login. In my app, to check the state of whether they need to re-login, I count the available tokens with the identifier in didFinishLaunchingWithOptions, like so:

   NSArray *oauth_tokens = [[NXOAuth2AccountStore sharedStore] accountsWithAccountType:@"my.account.flavor.identifier"];
    if([oauth_tokens count] == 0) {

        [self showSignupOrLogin];

    } else {
       [self showMainApp];
    }

From what I can tell, a user is in a state where the app is closed, and when it fires to life, it shows the signup/login flow instead of the main flow. However looking in the backend database, they have a valid access token, and the server has sent them no 401's.

I am setting up the NXOAuth2 library as such in my app deligates init method:

    [[NXOAuth2AccountStore sharedStore] setClientID:@"kkjjkkjjkkjjkkjjkkjjkkjjkkjjkkjj"
                                             secret:@"jkkjjkkjjkkjjkkjjkkjjkkjjkkjjkk"
                                   authorizationURL:[NSURL URLWithString:[NSString stringWithFormat:@"%@/api/v1/oauth2/authorize", HCApiBaseURL]]
                                           tokenURL:[NSURL URLWithString:[NSString stringWithFormat:@"%@/api/v1/oauth2/token", HCApiBaseURL]]
                                        redirectURL:[NSURL URLWithString:@"myapp://oauthreturn"]
                                     forAccountType:@"my.account.flavor.identifier"];

When a user taps login in my app, this is called:

    [[NXOAuth2AccountStore sharedStore] requestAccessToAccountWithType:@"my.account.flavor.identifier"
                                                              username:[username stringByTrimmingCharactersInSet:trimSet]
                                                              password:[password stringByTrimmingCharactersInSet:trimSet]];

Currently I am issuing access tokens with no expiration date. This is simply because I am using signed request functionality of NXOAuth2, so I need to build the refresh token mechanism myself, as I am not relying on NXOAuth2 to handle the requests. (I use rest kit, so..) I hope to soon implement refresh tokens, which may or may not help this issue.

I am more than happy to provide more information/code, and try different debugging strategies. I wish this was happening in the simulator. Unfortunately I have personally been unable to reproduce at all, but the number of users is troubling that have this. Some of them have crash reporting enabled and not seeing any crashes at all either.

Is there any case where keychain data would go missing? The users that have this happen it happens sporadically, they don't appear to have upgraded the app recently or restarted their phone or anything.

Thanks for any light you can shed. Very much appreciated.

cowboyrushforth commented 9 years ago

Another piece of data. My app originally launched with 1.2.2 of this library. I had on a subsequent release updated to 1.2.8, this started happening. (could be totally unrelated) So I rolled back to 1.2.5, and it still happens. If you think its worthwhile I can rollback to 1.2.2 I think. Would love to use 1.2.8 though as there was some nice compile warnings that disappeared.

Note: I updated the title of this issue, as I made some progress eliminating some confusion. Ignore the old title, thanks.

cowboyrushforth commented 9 years ago

Hi, small update. Been continuing to dig into this one. I have some beta testers running with 1.2.4 (rolled back from 1.2.5) and the issue did not go away. Too many code changes would be required to rollback to 1.2.2 so that was not tested.

In hearing more feedback from users, one thing stuck out, a user was presented the login screen randomly one morning, and they decided to exit the app. Upon existing they realized from test flight that a new version was available. Upon installing this, and starting it, they went right in without issue.

This leads me to believe that the token is not disappearing, but instead, the app gets into a state where it thinks it is.

After brainstorming, I am now trying this:

https://github.com/cowboyrushforth/OAuth2Client/commit/94cbdb8d004197c0d8dbd530e6d1984c3bc28a42

My theory is that the app is started from a cold start by a background process (could be push, or similar) and the app's keychain may be inaccessible, so it starts up and paints the login view. Where as from a cold start, in an unlocked state, the keychain is probably readable, and so it does not show the login screen.

Hopefully this helps, will report back. Not sure if this would be ideal for everyone as it leaves the keychain in a less secure state for sure. For my specific app its really not sensitive data too much, and there is a lot of background functionality, so I would want to keep it as open as possible. If this works I could make another commit to make this settable and create a PR.

emersonku commented 9 years ago

Hi cowboy - my setup is very similar to yours (using this library for authentication and Restkit for resource access). I'm struggling on building the token refresh. Can you suggest how you did it?

Many thanks!

cowboyrushforth commented 9 years ago

was just writing a reply to your issue, cheers

emersonku commented 9 years ago

Thanks!