Skip to content

Commit

Permalink
Invalidate session only if access token errors when extending token
Browse files Browse the repository at this point in the history
Summary:
Currently when there is a Platform API call that has any error or if there is a
connection error, e.g. timeout, the access token is invalidated. This was due
to
diff D378197 where any errors call invalidSession. To fix this narrow the
invalidate call to the scenario where the access token has expired when an API
call
is made.

For now, only REST API error codes allow us to easily identify an invalid token
(error code 190). A separate diff will be implemented by the Platform team
to provide an error code for Graph API auth token errors.

Test Plan:
Case #1
1/ Login
2/ Tap Graph API
3/ Tap Get your friends
4/ Go off LAN and WiFi
5/ Tap Get your friends

Case #2
1/ Turn WiFi back on
2/ Tap Get your friends
3/ See friends list
4/ Go back to get to Graph API page
5/ Remove app from https://www.facebook.com/settings/?tab=applications
6/ Tap Get your friends

Case #3
1/ Temporarily modified code to force a Graph API error
     //[[delegate facebook] requestWithGraphPath:@"me" andParams:params
andDelegate:self];
     [[delegate facebook] requestWithGraphPath:@"000000" andParams:params
andDelegate:self];
2/ Tap Graph API
3/ Tap Get your information

Before this change, here were the results
Case #1: Session expires message, which is incorrect behavior
Case #2: Session expires message, which is correct behavior
Case #3: Session expires message, which is incorrect behavior

After this change, here are the results
Case #1: Get the Hackbook "Oops something went wrong" message, which is desired
behavior
Case #2: Get the Hackbook "Oops something went wrong" message, which is not
ideal, user should also be logged out
Case #3: Get the Hackbook "Oops something went wrong" message, which is desired
behavior

Reviewers: yariv, jimbru, brent, toddkrabach, jonathan

Reviewed By: yariv

CC: lshepard, selekman, beau, bgolub, danmuriello, jacl

Differential Revision: https://phabricator.fb.com/D402481

Revert Plan: OK

Task ID: 900407
  • Loading branch information
Christine Abernathy authored and Christine Abernathy committed Feb 7, 2012
1 parent 4aa2a66 commit daed6b2
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 13 deletions.
2 changes: 2 additions & 0 deletions src/FBRequest.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ typedef NSUInteger FBRequestState;
NSMutableData* _responseText;
FBRequestState _state;
NSError* _error;
BOOL _sessionDidExpire;
}


Expand All @@ -64,6 +65,7 @@ typedef NSUInteger FBRequestState;
@property(nonatomic,retain) NSURLConnection* connection;
@property(nonatomic,retain) NSMutableData* responseText;
@property(nonatomic,readonly) FBRequestState state;
@property(nonatomic,readonly) BOOL sessionDidExpire;

/**
* Error returned by the server in case of request's failure (or nil otherwise).
Expand Down
16 changes: 9 additions & 7 deletions src/FBRequest.m
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@
static NSString* kUserAgent = @"FacebookConnect";
static NSString* kStringBoundary = @"3i2ndDfv2rTHiSisAbouNdArYfORhtTPEefj3q2f";
static const int kGeneralErrorCode = 10000;
static const int kRESTAPIAccessTokenErrorCode = 190;

static const NSTimeInterval kTimeoutInterval = 180.0;

///////////////////////////////////////////////////////////////////////////////////////////////////

@interface FBRequest ()
@property (nonatomic,readwrite) FBRequestState state;
@property (nonatomic,readwrite) BOOL sessionDidExpire;
@end

@implementation FBRequest
Expand All @@ -41,6 +43,7 @@ @implementation FBRequest
connection = _connection,
responseText = _responseText,
state = _state,
sessionDidExpire = _sessionDidExpire,
error = _error;
//////////////////////////////////////////////////////////////////////////////////////////////////
// class public
Expand Down Expand Up @@ -239,10 +242,12 @@ - (id)parseJsonResponse:(NSData *)data error:(NSError **)error {
* fails with error
*/
- (void)failWithError:(NSError *)error {
if ([error code] == kRESTAPIAccessTokenErrorCode) {
self.sessionDidExpire = YES;
}
if ([_delegate respondsToSelector:@selector(request:didFailWithError:)]) {
[_delegate request:self didFailWithError:error];
}
self.state = kFBRequestStateError;
}

/*
Expand Down Expand Up @@ -313,6 +318,7 @@ - (void)connect {

_connection = [[NSURLConnection alloc] initWithRequest:request delegate:self];
self.state = kFBRequestStateLoading;
self.sessionDidExpire = NO;
}

/**
Expand Down Expand Up @@ -355,19 +361,15 @@ - (void)connectionDidFinishLoading:(NSURLConnection *)connection {

self.responseText = nil;
self.connection = nil;

if (self.state != kFBRequestStateError) {
self.state = kFBRequestStateComplete;
}
self.state = kFBRequestStateComplete;
}

- (void)connection:(NSURLConnection *)connection didFailWithError:(NSError *)error {
[self failWithError:error];

self.responseText = nil;
self.connection = nil;

self.state = kFBRequestStateError;
self.state = kFBRequestStateComplete;
}

@end
12 changes: 6 additions & 6 deletions src/Facebook.m
Original file line number Diff line number Diff line change
Expand Up @@ -182,13 +182,13 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N
if (context == finishedContext) {
FBRequest* _request = (FBRequest*)object;
FBRequestState requestState = [_request state];
if (requestState == kFBRequestStateError) {
[self invalidateSession];
if ([self.sessionDelegate respondsToSelector:@selector(fbSessionInvalidated)]) {
[self.sessionDelegate fbSessionInvalidated];
if (requestState == kFBRequestStateComplete) {
if ([_request sessionDidExpire]) {
[self invalidateSession];
if ([self.sessionDelegate respondsToSelector:@selector(fbSessionInvalidated)]) {
[self.sessionDelegate fbSessionInvalidated];
}
}
}
if (requestState == kFBRequestStateComplete || requestState == kFBRequestStateError) {
[_request removeObserver:self forKeyPath:requestFinishedKeyPath];
[_requests removeObject:_request];
}
Expand Down

1 comment on commit daed6b2

@jasongregori
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I applaud this commit. Thank you @caabernathy!

Please sign in to comment.