Add support for GTMAppAuth 5 on macOS (#522)

Co-authored-by: Brianna Morales <74382627+brnnmrls@users.noreply.github.com>

GoogleSignIn/Sources/GIDAuthStateMigration.h

@@ -27,8 +27,7 @@ NS_ASSUME_NONNULL_BEGIN
 /// Creates an instance of this migration type with the keychain storage wrapper it will use.
 - (instancetype)initWithKeychainStore:(GTMKeychainStore *)keychainStore NS_DESIGNATED_INITIALIZER;
 
-/// Perform a one-time migration for auth state saved by GPPSignIn 1.x or GIDSignIn 1.0 - 4.x to the
-/// GTMAppAuth storage introduced in GIDSignIn 5.0.
+/// Perform necessary migrations from legacy auth state storage to most recent GTMAppAuth version.
 - (void)migrateIfNeededWithTokenURL:(NSURL *)tokenURL
                        callbackPath:(NSString *)callbackPath
                        keychainName:(NSString *)keychainName

GoogleSignIn/Sources/GIDAuthStateMigration.m

@@ -26,8 +26,12 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-// User preference key to detect whether or not the migration check has been performed.
-static NSString *const kMigrationCheckPerformedKey = @"GID_MigrationCheckPerformed";
+// User preference key to detect whether or not the migration to GTMAppAuth has been performed.
+static NSString *const kGTMAppAuthMigrationCheckPerformedKey = @"GID_MigrationCheckPerformed";
+
+// User preference key to detect whether or not the data protected migration has been performed.
+static NSString *const kDataProtectedMigrationCheckPerformedKey =
+                                                      @"GID_DataProtectedMigrationCheckPerformed";
 
 // Keychain account used to store additional state in SDKs previous to v5, including GPPSignIn.
 static NSString *const kOldKeychainAccount = @"GooglePlus";
@@ -63,32 +67,85 @@ static NSString *const kFingerprintService = @"fingerprint";
                        callbackPath:(NSString *)callbackPath
                        keychainName:(NSString *)keychainName
                      isFreshInstall:(BOOL)isFreshInstall {
+  // If this is a fresh install, take no action and mark the migration checks as having been
+  // performed.
+  if (isFreshInstall) {
+    NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults];
+#if TARGET_OS_OSX || TARGET_OS_MACCATALYST
+    [defaults setBool:YES forKey:kDataProtectedMigrationCheckPerformedKey];
+#elif TARGET_OS_IOS
+    [defaults setBool:YES forKey:kGTMAppAuthMigrationCheckPerformedKey];
+#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST
+    return;
+  }
+
+#if TARGET_OS_OSX || TARGET_OS_MACCATALYST
+  [self performDataProtectedMigrationIfNeeded];
+#elif TARGET_OS_IOS
+  [self performGIDMigrationIfNeededWithTokenURL:tokenURL
+                                   callbackPath:callbackPath
+                                   keychainName:keychainName];
+#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST
+}
+
+#if TARGET_OS_OSX || TARGET_OS_MACCATALYST
+// Migrate from the fileBasedKeychain to dataProtectedKeychain with GTMAppAuth 5.0.
+- (void)performDataProtectedMigrationIfNeeded {
   // See if we've performed the migration check previously.
   NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults];
-  if ([defaults boolForKey:kMigrationCheckPerformedKey]) {
+  if ([defaults boolForKey:kDataProtectedMigrationCheckPerformedKey]) {
     return;
   }
 
-  // If this is not a fresh install, attempt to migrate state.  If this is a fresh install, take no
-  // action and go on to mark the migration check as having been performed.
-  if (!isFreshInstall) {
-    // Attempt migration
-    GTMAuthSession *authSession =
-        [self extractAuthSessionWithTokenURL:tokenURL callbackPath:callbackPath];
-
-    // If migration was successful, save our migrated state to the keychain.
-    if (authSession) {
-      NSError *err;
-      [self.keychainStore saveAuthSession:authSession error:&err];
-      // If we're unable to save to the keychain, return without marking migration performed.
-      if (err) {
-        return;
-      };
-    }
+  GTMKeychainAttribute *fileBasedKeychain = [GTMKeychainAttribute useFileBasedKeychain];
+  NSSet *attributes = [NSSet setWithArray:@[fileBasedKeychain]];
+  GTMKeychainStore *keychainStoreLegacy =
+      [[GTMKeychainStore alloc] initWithItemName:self.keychainStore.itemName
+                              keychainAttributes:attributes];
+  GTMAuthSession *authSession = [keychainStoreLegacy retrieveAuthSessionWithError:nil];
+  // If migration was successful, save our migrated state to the keychain.
+  if (authSession) {
+    NSError *err;
+    [self.keychainStore saveAuthSession:authSession error:&err];
+    // If we're unable to save to the keychain, return without marking migration performed.
+    if (err) {
+      return;
+    };
+    [keychainStoreLegacy removeAuthSessionWithError:nil];
+  }
+
+  // Mark the migration check as having been performed.
+  [defaults setBool:YES forKey:kDataProtectedMigrationCheckPerformedKey];
+}
+
+#elif TARGET_OS_IOS
+// Migrate from GPPSignIn 1.x or GIDSignIn 1.0 - 4.x to the GTMAppAuth storage introduced in
+// GIDSignIn 5.0.
+- (void)performGIDMigrationIfNeededWithTokenURL:(NSURL *)tokenURL
+                                   callbackPath:(NSString *)callbackPath
+                                   keychainName:(NSString *)keychainName {
+  // See if we've performed the migration check previously.
+  NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults];
+  if ([defaults boolForKey:kGTMAppAuthMigrationCheckPerformedKey]) {
+    return;
+  }
+
+  // Attempt migration
+  GTMAuthSession *authSession =
+      [self extractAuthSessionWithTokenURL:tokenURL callbackPath:callbackPath];
+
+  // If migration was successful, save our migrated state to the keychain.
+  if (authSession) {
+    NSError *err;
+    [self.keychainStore saveAuthSession:authSession error:&err];
+    // If we're unable to save to the keychain, return without marking migration performed.
+    if (err) {
+      return;
+    };
   }
 
   // Mark the migration check as having been performed.
-  [defaults setBool:YES forKey:kMigrationCheckPerformedKey];
+  [defaults setBool:YES forKey:kGTMAppAuthMigrationCheckPerformedKey];
 }
 
 // Returns a |GTMAuthSession| object containing any old auth state or |nil| if none
@@ -189,6 +246,7 @@ static NSString *const kFingerprintService = @"fingerprint";
   NSString *password = [[NSString alloc] initWithData:passwordData encoding:NSUTF8StringEncoding];
   return password;
 }
+#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST
 
 @end
 

GoogleSignIn/Sources/GIDSignIn.m

@@ -21,6 +21,7 @@
 #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDProfileData.h"
 #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignInResult.h"
 
+#import "GoogleSignIn/Sources/GIDAuthStateMigration.h"
 #import "GoogleSignIn/Sources/GIDEMMSupport.h"
 #import "GoogleSignIn/Sources/GIDSignInInternalOptions.h"
 #import "GoogleSignIn/Sources/GIDSignInPreferences.h"
@@ -31,7 +32,6 @@
 #import <AppCheckCore/GACAppCheckToken.h>
 #import "GoogleSignIn/Sources/GIDAppCheck/Implementations/GIDAppCheck.h"
 #import "GoogleSignIn/Sources/GIDAppCheck/UI/GIDActivityIndicatorViewController.h"
-#import "GoogleSignIn/Sources/GIDAuthStateMigration.h"
 #import "GoogleSignIn/Sources/GIDEMMErrorHandler.h"
 #import "GoogleSignIn/Sources/GIDTimedLoader/GIDTimedLoader.h"
 #endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST
@@ -560,15 +560,13 @@ static NSString *const kConfigOpenIDRealmKey = @"GIDOpenIDRealm";
                              initWithAuthorizationEndpoint:[NSURL URLWithString:authorizationEnpointURL]
                              tokenEndpoint:[NSURL URLWithString:tokenEndpointURL]];
     _keychainStore = keychainStore;
-#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST
-    // Perform migration of auth state from old (before 5.0) versions of the SDK if needed.
+    // Perform migration of auth state from old versions of the SDK if needed.
     GIDAuthStateMigration *migration =
         [[GIDAuthStateMigration alloc] initWithKeychainStore:_keychainStore];
     [migration migrateIfNeededWithTokenURL:_appAuthConfiguration.tokenEndpoint
                               callbackPath:kBrowserCallbackPath
                               keychainName:kGTMAppAuthKeychainName
                             isFreshInstall:isFreshInstall];
-#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST
   }
   return self;
 }

GoogleSignIn/Tests/Unit/GIDAuthStateMigrationTest.m

@@ -16,6 +16,9 @@
 
 #import "GoogleSignIn/Sources/GIDAuthStateMigration.h"
 #import "GoogleSignIn/Sources/GIDSignInCallbackSchemes.h"
+#if TARGET_OS_OSX || TARGET_OS_MACCATALYST
+#import "GoogleSignIn/Tests/Unit/OIDAuthState+Testing.h"
+#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST
 
 @import GTMAppAuth;
 
@@ -49,7 +52,9 @@ static NSString *const kFinalPersistenceString =
 static NSString *const kRedirectURI =
     @"com.googleusercontent.apps.223520599684-kg64hfn0h950oureqacja2fltg00msv3:/callback/path";
 
-static NSString *const kMigrationCheckPerformedKey = @"GID_MigrationCheckPerformed";
+static NSString *const kGTMAppAuthMigrationCheckPerformedKey = @"GID_MigrationCheckPerformed";
+static NSString *const kDataProtectedMigrationCheckPerformedKey =
+                                                      @"GID_DataProtectedMigrationCheckPerformed";
 static NSString *const kFingerprintService = @"fingerprint";
 
 NS_ASSUME_NONNULL_BEGIN
@@ -79,6 +84,9 @@ NS_ASSUME_NONNULL_BEGIN
   id _mockNSBundle;
   id _mockGIDSignInCallbackSchemes;
   id _mockGTMOAuth2Compatibility;
+#if TARGET_OS_OSX || TARGET_OS_MACCATALYST
+  id _realLegacyGTMKeychainStore;
+#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST
 }
 
 - (void)setUp {
@@ -92,6 +100,12 @@ NS_ASSUME_NONNULL_BEGIN
   _mockNSBundle = OCMStrictClassMock([NSBundle class]);
   _mockGIDSignInCallbackSchemes = OCMStrictClassMock([GIDSignInCallbackSchemes class]);
   _mockGTMOAuth2Compatibility = OCMStrictClassMock([GTMOAuth2Compatibility class]);
+#if TARGET_OS_OSX || TARGET_OS_MACCATALYST
+  GTMKeychainAttribute *fileBasedKeychain = [GTMKeychainAttribute useFileBasedKeychain];
+  NSSet *attributes = [NSSet setWithArray:@[fileBasedKeychain]];
+  _realLegacyGTMKeychainStore = [[GTMKeychainStore alloc] initWithItemName:kKeychainName
+                                                        keychainAttributes:attributes];
+#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST
 }
 
 - (void)tearDown {
@@ -111,20 +125,30 @@ NS_ASSUME_NONNULL_BEGIN
   [_mockGIDSignInCallbackSchemes stopMocking];
   [_mockGTMOAuth2Compatibility verify];
   [_mockGTMOAuth2Compatibility stopMocking];
+#if TARGET_OS_OSX || TARGET_OS_MACCATALYST
+  [_realLegacyGTMKeychainStore removeAuthSessionWithError:nil];
+#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST
 
   [super tearDown];
 }
 
 #pragma mark - Tests
 
-- (void)testMigrateIfNeeded_NoPreviousMigration {
+#if TARGET_OS_OSX || TARGET_OS_MACCATALYST
+- (void)testMigrateIfNeeded_NoPreviousMigration_DataProtectedMigration {
   [[[_mockUserDefaults stub] andReturn:_mockUserDefaults] standardUserDefaults];
-  [[[_mockUserDefaults expect] andReturnValue:@NO] boolForKey:kMigrationCheckPerformedKey];
-  [[_mockUserDefaults expect] setBool:YES forKey:kMigrationCheckPerformedKey];
+  [[[_mockUserDefaults expect] andReturnValue:@NO] boolForKey:kDataProtectedMigrationCheckPerformedKey];
+  [[_mockUserDefaults expect] setBool:YES forKey:kDataProtectedMigrationCheckPerformedKey];
 
   [[_mockGTMKeychainStore expect] saveAuthSession:OCMOCK_ANY error:OCMArg.anyObjectRef];
+  [[[_mockGTMKeychainStore expect] andReturn:kKeychainName] itemName];
 
-  [self setUpCommonExtractAuthorizationMocksWithFingerPrint:kSavedFingerprint];
+  // set the auth session that will be migrated
+  OIDAuthState *authState = [OIDAuthState testInstance];
+  GTMAuthSession *authSession = [[GTMAuthSession alloc] initWithAuthState:authState];
+  NSError *err;
+  [_realLegacyGTMKeychainStore saveAuthSession:authSession error:&err];
+  XCTAssertNil(err);
 
   GIDAuthStateMigration *migration =
       [[GIDAuthStateMigration alloc] initWithKeychainStore:_mockGTMKeychainStore];
@@ -132,12 +156,24 @@ NS_ASSUME_NONNULL_BEGIN
                             callbackPath:kCallbackPath
                             keychainName:kKeychainName
                           isFreshInstall:NO];
+
+  // verify that the auth session was removed during migration
+  XCTAssertNil([_realLegacyGTMKeychainStore retrieveAuthSessionWithError:nil]);
 }
 
-- (void)testMigrateIfNeeded_HasPreviousMigration {
+- (void)testMigrateIfNeeded_KeychainFailure_DataProtectedMigration {
   [[[_mockUserDefaults stub] andReturn:_mockUserDefaults] standardUserDefaults];
-  [[[_mockUserDefaults expect] andReturnValue:@YES] boolForKey:kMigrationCheckPerformedKey];
-  [[_mockUserDefaults reject] setBool:YES forKey:kMigrationCheckPerformedKey];
+  [[[_mockUserDefaults expect] andReturnValue:@NO] boolForKey:kDataProtectedMigrationCheckPerformedKey];
+
+  NSError *keychainSaveError = [NSError new];
+  OCMStub([_mockGTMKeychainStore saveAuthSession:OCMOCK_ANY error:[OCMArg setTo:keychainSaveError]]);
+
+  [[[_mockGTMKeychainStore expect] andReturn:kKeychainName] itemName];
+  OIDAuthState *authState = [OIDAuthState testInstance];
+  GTMAuthSession *authSession = [[GTMAuthSession alloc] initWithAuthState:authState];
+  NSError *err;
+  [_realLegacyGTMKeychainStore saveAuthSession:authSession error:&err];
+  XCTAssertNil(err);
 
   GIDAuthStateMigration *migration =
       [[GIDAuthStateMigration alloc] initWithKeychainStore:_mockGTMKeychainStore];
@@ -145,14 +181,16 @@ NS_ASSUME_NONNULL_BEGIN
                             callbackPath:kCallbackPath
                             keychainName:kKeychainName
                           isFreshInstall:NO];
+  XCTAssertNotNil([_realLegacyGTMKeychainStore retrieveAuthSessionWithError:nil]);
 }
 
-- (void)testMigrateIfNeeded_KeychainFailure {
+#else
+- (void)testMigrateIfNeeded_NoPreviousMigration_GTMAppAuthMigration {
   [[[_mockUserDefaults stub] andReturn:_mockUserDefaults] standardUserDefaults];
-  [[[_mockUserDefaults expect] andReturnValue:@NO] boolForKey:kMigrationCheckPerformedKey];
+  [[[_mockUserDefaults expect] andReturnValue:@NO] boolForKey:kGTMAppAuthMigrationCheckPerformedKey];
+  [[_mockUserDefaults expect] setBool:YES forKey:kGTMAppAuthMigrationCheckPerformedKey];
 
-  NSError *keychainSaveError = [NSError new];
-  OCMStub([_mockGTMKeychainStore saveAuthSession:OCMOCK_ANY error:[OCMArg setTo:keychainSaveError]]);
+  [[_mockGTMKeychainStore expect] saveAuthSession:OCMOCK_ANY error:OCMArg.anyObjectRef];
 
   [self setUpCommonExtractAuthorizationMocksWithFingerPrint:kSavedFingerprint];
 
@@ -164,18 +202,21 @@ NS_ASSUME_NONNULL_BEGIN
                           isFreshInstall:NO];
 }
 
-- (void)testMigrateIfNeeded_isFreshInstall {
+- (void)testMigrateIfNeeded_KeychainFailure_GTMAppAuthMigration {
   [[[_mockUserDefaults stub] andReturn:_mockUserDefaults] standardUserDefaults];
-  [[[_mockUserDefaults expect] andReturnValue:@NO]
-      boolForKey:kMigrationCheckPerformedKey];
-  [[_mockUserDefaults expect] setBool:YES forKey:kMigrationCheckPerformedKey];
+  [[[_mockUserDefaults expect] andReturnValue:@NO] boolForKey:kGTMAppAuthMigrationCheckPerformedKey];
+
+  NSError *keychainSaveError = [NSError new];
+  OCMStub([_mockGTMKeychainStore saveAuthSession:OCMOCK_ANY error:[OCMArg setTo:keychainSaveError]]);
+
+  [self setUpCommonExtractAuthorizationMocksWithFingerPrint:kSavedFingerprint];
 
   GIDAuthStateMigration *migration =
       [[GIDAuthStateMigration alloc] initWithKeychainStore:_mockGTMKeychainStore];
   [migration migrateIfNeededWithTokenURL:[NSURL URLWithString:kTokenURL]
                             callbackPath:kCallbackPath
                             keychainName:kKeychainName
-                          isFreshInstall:YES];
+                          isFreshInstall:NO];
 }
 
 - (void)testExtractAuthorization {
@@ -201,6 +242,43 @@ NS_ASSUME_NONNULL_BEGIN
 
   XCTAssertNotNil(authorization);
 }
+#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST
+
+- (void)testMigrateIfNeeded_HasPreviousMigration {
+  [[[_mockUserDefaults stub] andReturn:_mockUserDefaults] standardUserDefaults];
+#if TARGET_OS_OSX || TARGET_OS_MACCATALYST
+  [[[_mockUserDefaults expect] andReturnValue:@YES] boolForKey:kDataProtectedMigrationCheckPerformedKey];
+  [[_mockUserDefaults reject] setBool:YES forKey:kDataProtectedMigrationCheckPerformedKey];
+#else
+  [[[_mockUserDefaults expect] andReturnValue:@YES] boolForKey:kGTMAppAuthMigrationCheckPerformedKey];
+  [[_mockUserDefaults reject] setBool:YES forKey:kGTMAppAuthMigrationCheckPerformedKey];
+#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST
+
+  GIDAuthStateMigration *migration =
+      [[GIDAuthStateMigration alloc] initWithKeychainStore:_mockGTMKeychainStore];
+  [migration migrateIfNeededWithTokenURL:[NSURL URLWithString:kTokenURL]
+                            callbackPath:kCallbackPath
+                            keychainName:kKeychainName
+                          isFreshInstall:NO];
+}
+
+- (void)testMigrateIfNeeded_isFreshInstall {
+  [[[_mockUserDefaults stub] andReturn:_mockUserDefaults] standardUserDefaults];
+#if TARGET_OS_OSX || TARGET_OS_MACCATALYST
+  [[_mockUserDefaults expect] setBool:YES forKey:kDataProtectedMigrationCheckPerformedKey];
+#else
+  [[_mockUserDefaults expect] setBool:YES forKey:kGTMAppAuthMigrationCheckPerformedKey];
+#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST
+
+  GIDAuthStateMigration *migration =
+      [[GIDAuthStateMigration alloc] initWithKeychainStore:_mockGTMKeychainStore];
+  [migration migrateIfNeededWithTokenURL:[NSURL URLWithString:kTokenURL]
+                            callbackPath:kCallbackPath
+                            keychainName:kKeychainName
+                          isFreshInstall:YES];
+}
+
+
 
 #pragma mark - Helpers
 

README.md

@@ -47,15 +47,15 @@ If you would like to see a Swift example, take a look at
 Google Sign-In allows your users to sign-in to your native macOS app using their Google account
 and default browser.  When building for macOS, the `signInWithConfiguration:` and `addScopes:`
 methods take a `presentingWindow:` parameter in place of `presentingViewController:`.  Note that
-in order for your macOS app to store credientials via the Keychain on macOS, you will need to
-[sign your app](https://developer.apple.com/support/code-signing/).
+in order for your macOS app to store credentials via the Keychain on macOS, you will need to add
+`$(AppIdentifierPrefix)$(CFBundleIdentifier)` to its keychain access group.
 
 ### Mac Catalyst
 
 Google Sign-In also supports iOS apps that are built for macOS via
 [Mac Catalyst](https://developer.apple.com/mac-catalyst/).  In order for your Mac Catalyst app
-to store credientials via the Keychain on macOS, you will need to
-[sign your app](https://developer.apple.com/support/code-signing/).
+to store credentials via the Keychain on macOS, you will need to add
+`$(AppIdentifierPrefix)$(CFBundleIdentifier)` to its keychain access group.
 
 ## Using the Google Sign-In Button
 

Samples/Swift/DaysUntilBirthday/README.md

@@ -26,25 +26,6 @@ open DaysUntilBirthday.xcodeproj
 ```
 2. Run the `DaysUntilBirthday (iOS)` or `DaysUntilBirthday (macOS)` target.
 
-## A Note on running the macOS Sample
-
-If you are running the `DaysUntilBirthday` sample app on macOS, you may see an
-error like the below:
-
-```
-Error! Optional(Error Domain=com.google.GIDSignIn Code=-2 "keychain error" UserInfo={NSLocalizedDescription=keychain error})
-```
-
-This error is related to the signing of the sample app.
-You have two choices to remedy the problem.
-
-1. We suggest that you manually manage the signing of the macOS sample app so
-that you can provide a provisioning profile. Make sure to select a profile
-that is able to sign macOS apps.
-2. If you do not have the correct provisioning profile, and are unable to
-generate one, then you can add the ["Keychain Sharing" capability](https://developer.apple.com/documentation/xcode/configuring-keychain-sharing)
-to the `DaysUntilBirthday (macOS)` target as a workaround.
-
 ## Integration Tests
 
 We run integration tests on the `DaysUntilBirthday(iOS)` sample app.

Samples/Swift/DaysUntilBirthday/macOS/DaysUntilBirthdayOnMac.entitlements

@@ -2,6 +2,10 @@
 <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
 <plist version="1.0">
 <dict>
+	<key>keychain-access-groups</key>
+	<array>
+		<string>$(AppIdentifierPrefix)$(CFBundleIdentifier)</string>
+	</array>
 	<key>com.apple.security.app-sandbox</key>
 	<true/>
 	<key>com.apple.security.files.user-selected.read-only</key>