Send token revocation request as POST with form-encoded body

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<NSURLQueryItem *> *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<NSURLQueryItem *> *bodyItems = [NSMutableArray array];
+  [bodyItems addObject:[NSURLQueryItem queryItemWithName:kRevokeTokenParameter value:token]];
   NSDictionary<NSString *, NSString *> *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<NSString *, NSString *> *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<GTMSessionFetcherServiceProtocol> fetcherService = authorization.fetcherService;

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;
 

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;

GoogleSignIn/Tests/Unit/GIDSignInTest.m

@@ -61,7 +61,6 @@
 #import <AppAuth/OIDGrantTypes.h>
 #import <AppAuth/OIDTokenRequest.h>
 #import <AppAuth/OIDTokenResponse.h>
-#import <AppAuth/OIDURLQueryComponent.h>
 
 #if TARGET_OS_IOS || TARGET_OS_MACCATALYST
 #import <AppAuth/OIDAuthorizationService+IOS.h>
@@ -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<NSString *, NSString *> *)fetchedBodyParameters {
+  NSData *body = [self fetchedHTTPBody];
+  NSString *bodyString = [[NSString alloc] initWithData:body encoding:NSUTF8StringEncoding];
+  NSURLComponents *components = [[NSURLComponents alloc] init];
+  components.percentEncodedQuery = bodyString;
+  NSMutableDictionary<NSString *, NSString *> *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<NSString *, NSObject<NSCopying> *> *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<NSString *, NSString *> *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.