diff --git a/GoogleSignIn/Sources/GIDSignIn.m b/GoogleSignIn/Sources/GIDSignIn.m index f910e72d..7dc07d50 100644 --- a/GoogleSignIn/Sources/GIDSignIn.m +++ b/GoogleSignIn/Sources/GIDSignIn.m @@ -95,6 +95,12 @@ // The scheme used for requests to Google's servers. static NSString *const kHTTPSScheme = @"https"; +// The HTTP method used to revoke a token, as required by RFC 7009 section 2.1. +static NSString *const kHTTPMethodPost = @"POST"; + +// The content type of a form-encoded request body. +static NSString *const kContentTypeFormURLEncoded = @"application/x-www-form-urlencoded"; + // Expected path in the URL scheme to be handled. static NSString *const kBrowserCallbackPath = @"/oauth2callback"; @@ -596,21 +602,28 @@ - (void)disconnectWithCompletion:(nullable GIDDisconnectCompletion)completion { revokeURLComponents.host = [GIDSignInPreferences googleAuthorizationServer]; revokeURLComponents.path = kRevokeTokenPath; - NSMutableArray *queryItems = [NSMutableArray array]; - [queryItems addObject:[NSURLQueryItem queryItemWithName:kRevokeTokenParameter value:token]]; + // The token revocation endpoint expects the token in the body of a POST request (RFC 7009 + // section 2.1). Build a form-encoded body, reusing the same encoding as a URL query so a "+" + // in the token survives form decoding on the server. + NSMutableArray *bodyItems = [NSMutableArray array]; + [bodyItems addObject:[NSURLQueryItem queryItemWithName:kRevokeTokenParameter value:token]]; NSDictionary *loggingParameters = [GIDSignInPreferences loggingParameters]; for (NSString *name in [loggingParameters.allKeys sortedArrayUsingSelector:@selector(compare:)]) { - [queryItems addObject:[NSURLQueryItem queryItemWithName:name + [bodyItems addObject:[NSURLQueryItem queryItemWithName:name value:loggingParameters[name]]]; } - revokeURLComponents.queryItems = queryItems; - GIDPercentEncodePlusInQuery(revokeURLComponents); + NSURLComponents *bodyComponents = [[NSURLComponents alloc] init]; + bodyComponents.queryItems = bodyItems; + GIDPercentEncodePlusInQuery(bodyComponents); + NSData *body = [bodyComponents.percentEncodedQuery dataUsingEncoding:NSUTF8StringEncoding]; [self startFetchURL:revokeURLComponents.URL - fromAuthState:authState - withComment:@"GIDSignIn: revoke tokens" - withCompletionHandler:^(NSData *data, NSError *error) { + method:kHTTPMethodPost + body:body + fromAuthState:authState + withComment:@"GIDSignIn: revoke tokens" + withCompletionHandler:^(NSData *data, NSError *error) { // Revoking an already revoked token seems always successful, which helps us here. if (!error) { [self signOut]; @@ -1169,9 +1182,11 @@ - (void)addDecodeIdTokenCallback:(GIDAuthFlow *)authFlow { ]; GIDPercentEncodePlusInQuery(infoURLComponents); [self startFetchURL:infoURLComponents.URL - fromAuthState:authState - withComment:@"GIDSignIn: fetch basic profile info" - withCompletionHandler:^(NSData *data, NSError *error) { + method:@"GET" + body:nil + fromAuthState:authState + withComment:@"GIDSignIn: fetch basic profile info" + withCompletionHandler:^(NSData *data, NSError *error) { if (data && !error) { NSError *jsonDeserializationError; NSDictionary *profileDict = @@ -1222,10 +1237,17 @@ - (void)addCompletionCallback:(GIDAuthFlow *)authFlow { } - (void)startFetchURL:(NSURL *)URL - fromAuthState:(OIDAuthState *)authState - withComment:(NSString *)comment - withCompletionHandler:(void (^)(NSData *, NSError *))handler { + method:(NSString *)method + body:(NSData *)body + fromAuthState:(OIDAuthState *)authState + withComment:(NSString *)comment +withCompletionHandler:(void (^)(NSData *, NSError *))handler { NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:URL]; + request.HTTPMethod = method; + if (body) { + request.HTTPBody = body; + [request setValue:kContentTypeFormURLEncoded forHTTPHeaderField:@"Content-Type"]; + } GTMSessionFetcher *fetcher; GTMAuthSession *authorization = [[GTMAuthSession alloc] initWithAuthState:authState]; id fetcherService = authorization.fetcherService; diff --git a/GoogleSignIn/Tests/Unit/GIDFakeFetcher.h b/GoogleSignIn/Tests/Unit/GIDFakeFetcher.h index c31c05eb..571a38d4 100644 --- a/GoogleSignIn/Tests/Unit/GIDFakeFetcher.h +++ b/GoogleSignIn/Tests/Unit/GIDFakeFetcher.h @@ -26,6 +26,12 @@ /// The URL of the fetching request. - (NSURL *)requestURL; +/// The HTTP method of the fetching request. +- (NSString *)requestHTTPMethod; + +/// The HTTP body of the fetching request. +- (NSData *)requestHTTPBody; + // Emulates server returning with data and/or error. - (void)didFinishWithData:(NSData *)data error:(NSError *)error; diff --git a/GoogleSignIn/Tests/Unit/GIDFakeFetcher.m b/GoogleSignIn/Tests/Unit/GIDFakeFetcher.m index 80712df0..fa05e585 100644 --- a/GoogleSignIn/Tests/Unit/GIDFakeFetcher.m +++ b/GoogleSignIn/Tests/Unit/GIDFakeFetcher.m @@ -20,12 +20,16 @@ @implementation GIDFakeFetcher { FetchCompletionHandler _handler; NSURL *_requestURL; + NSString *_requestHTTPMethod; + NSData *_requestHTTPBody; } - (instancetype)initWithRequest:(NSURLRequest *)request { self = [super initWithRequest:request configuration:nil]; if (self) { _requestURL = [[request URL] copy]; + _requestHTTPMethod = [request.HTTPMethod copy]; + _requestHTTPBody = [request.HTTPBody copy]; } return self; } @@ -65,6 +69,14 @@ - (NSURL *)requestURL { return _requestURL; } +- (NSString *)requestHTTPMethod { + return _requestHTTPMethod; +} + +- (NSData *)requestHTTPBody { + return _requestHTTPBody; +} + - (void)didFinishWithData:(NSData *)data error:(NSError *)error { FetchCompletionHandler handler = _handler; _handler = nil; diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index a98cdf25..3496fabe 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -61,7 +61,6 @@ #import #import #import -#import #if TARGET_OS_IOS || TARGET_OS_MACCATALYST #import @@ -1350,7 +1349,7 @@ - (void)testDisconnectNoCallback_accessToken { } // Verifies a token containing characters that are reserved in a URL query is percent-encoded -// in the revoke URL, so that it arrives at the server intact. +// in the revoke request body, so that it arrives at the server intact. - (void)testDisconnectNoCallback_tokenWithReservedCharacters { NSString *tokenWithReservedCharacters = @"token&with=reserved#characters"; [[[_authorization expect] andReturn:_authState] authState]; @@ -1383,22 +1382,14 @@ - (void)testDisconnectNoCallback_tokenWithPlusCharacter { XCTAssertEqualObjects([url host], @"accounts.google.com", @"host must match"); XCTAssertEqualObjects([url path], @"/o/oauth2/revoke", @"path must match"); - NSString *query = [[self fetchedURL] query]; - XCTAssertTrue([query containsString:@"token=token%2Bwith%2Bplus"], - @"'+' should be percent-encoded in the query string"); - XCTAssertFalse([query containsString:@"token=token+with+plus"], - @"'+' should not be literal in the query string"); + NSString *bodyString = [[NSString alloc] initWithData:[self fetchedHTTPBody] + encoding:NSUTF8StringEncoding]; + XCTAssertTrue([bodyString containsString:@"token=token%2Bwith%2Bplus"], + @"'+' should be percent-encoded in the body"); + XCTAssertFalse([bodyString containsString:@"token=token+with+plus"], + @"'+' should not be literal in the body"); - NSURLComponents *components = - [NSURLComponents componentsWithURL:[self fetchedURL] resolvingAgainstBaseURL:NO]; - NSURLQueryItem *tokenItem; - for (NSURLQueryItem *item in components.queryItems) { - if ([item.name isEqualToString:@"token"]) { - tokenItem = item; - break; - } - } - XCTAssertEqualObjects(tokenItem.value, tokenWithPlusCharacter); + XCTAssertEqualObjects([self fetchedBodyParameters][@"token"], tokenWithPlusCharacter); [self didFetch:nil error:nil]; XCTAssertTrue(_keychainRemoved, @"should clear saved keychain name"); @@ -1418,10 +1409,11 @@ - (void)testDisconnectNoCallback_tokenWithSpace { [[[_authorization expect] andReturn:_fetcherService] fetcherService]; [_signIn disconnectWithCompletion:nil]; - NSString *query = [[self fetchedURL] query]; - XCTAssertTrue([query containsString:@"token=token%20with%20space"], - @"a space should be percent-encoded in the query string"); - XCTAssertFalse([query containsString:@"+"], @"a space should never be encoded as '+'"); + NSString *bodyString = [[NSString alloc] initWithData:[self fetchedHTTPBody] + encoding:NSUTF8StringEncoding]; + XCTAssertTrue([bodyString containsString:@"token=token%20with%20space"], + @"a space should be percent-encoded in the body"); + XCTAssertFalse([bodyString containsString:@"+"], @"a space should never be encoded as '+'"); [self didFetch:nil error:nil]; XCTAssertTrue(_keychainRemoved, @"should clear saved keychain name"); @@ -1430,7 +1422,8 @@ - (void)testDisconnectNoCallback_tokenWithSpace { [_tokenResponse verify]; } -// Round-trip the revoke URL through OIDURLQueryComponent, a pretend server, to check "+" survives. +// Round-trip the revoke request body through form decoding, a pretend server, to check "+" +// survives. - (void)testDisconnectNoCallback_tokenWithPlusCharacterFormDecoded { NSString *tokenWithPlusCharacter = @"token+with+plus"; [[[_authorization expect] andReturn:_authState] authState]; @@ -1791,6 +1784,29 @@ - (NSURL *)fetchedURL { return [_fetcherService.fetchers[0] requestURL]; } +// Gets the HTTP method of the fetching request. +- (NSString *)fetchedHTTPMethod { + return [_fetcherService.fetchers[0] requestHTTPMethod]; +} + +// Gets the HTTP body of the fetching request. +- (NSData *)fetchedHTTPBody { + return [_fetcherService.fetchers[0] requestHTTPBody]; +} + +// Decodes the form-encoded HTTP body of the fetching request into a dictionary of parameters. +- (NSDictionary *)fetchedBodyParameters { + NSData *body = [self fetchedHTTPBody]; + NSString *bodyString = [[NSString alloc] initWithData:body encoding:NSUTF8StringEncoding]; + NSURLComponents *components = [[NSURLComponents alloc] init]; + components.percentEncodedQuery = bodyString; + NSMutableDictionary *parameters = [NSMutableDictionary dictionary]; + for (NSURLQueryItem *item in components.queryItems) { + parameters[item.name] = item.value; + } + return parameters; +} + // Emulates server returning the data as in JSON. - (void)didFetch:(id)dataObject error:(NSError *)error { NSData *data = nil; @@ -1817,14 +1833,14 @@ - (void)verifyAndRevokeToken:(NSString *)token XCTAssertEqualObjects([url scheme], @"https", @"scheme must match"); XCTAssertEqualObjects([url host], @"accounts.google.com", @"host must match"); XCTAssertEqualObjects([url path], @"/o/oauth2/revoke", @"path must match"); - OIDURLQueryComponent *queryComponent = [[OIDURLQueryComponent alloc] initWithURL:url]; - NSDictionary *> *params = queryComponent.dictionaryValue; - XCTAssertEqualObjects([params valueForKey:@"token"], token, - @"token parameter should match"); - XCTAssertEqualObjects([params valueForKey:kSDKVersionLoggingParameter], + XCTAssertNil([url query], @"revocation token must not be in the query string"); + XCTAssertEqualObjects([self fetchedHTTPMethod], @"POST", @"HTTP method must be POST"); + NSDictionary *params = [self fetchedBodyParameters]; + XCTAssertEqualObjects(params[@"token"], token, @"token parameter should match"); + XCTAssertEqualObjects(params[kSDKVersionLoggingParameter], [GIDSignInPreferences sdkVersion], @"SDK version logging parameter should match"); - XCTAssertEqualObjects([params valueForKey:kEnvironmentLoggingParameter], + XCTAssertEqualObjects(params[kEnvironmentLoggingParameter], [GIDSignInPreferences environment], @"Environment logging parameter should match"); // Emulate result back from server.