diff --git a/GoogleSignIn/Sources/GIDAuthStateMigration.h b/GoogleSignIn/Sources/GIDAuthStateMigration.h index 329be59f..738368fc 100644 --- a/GoogleSignIn/Sources/GIDAuthStateMigration.h +++ b/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 diff --git a/GoogleSignIn/Sources/GIDAuthStateMigration.m b/GoogleSignIn/Sources/GIDAuthStateMigration.m index f0827f03..4df01d11 100644 --- a/GoogleSignIn/Sources/GIDAuthStateMigration.m +++ b/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 @@ - (void)migrateIfNeededWithTokenURL:(NSURL *)tokenURL 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 @@ + (nullable NSString *)passwordForService:(NSString *)service { NSString *password = [[NSString alloc] initWithData:passwordData encoding:NSUTF8StringEncoding]; return password; } +#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST @end diff --git a/GoogleSignIn/Sources/GIDSignIn.m b/GoogleSignIn/Sources/GIDSignIn.m index 093e33d6..640761e1 100644 --- a/GoogleSignIn/Sources/GIDSignIn.m +++ b/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 #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 @@ - (instancetype)initWithKeychainStore:(GTMKeychainStore *)keychainStore { 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; } diff --git a/GoogleSignIn/Tests/Unit/GIDAuthStateMigrationTest.m b/GoogleSignIn/Tests/Unit/GIDAuthStateMigrationTest.m index 85ff7cca..6379fb4f 100644 --- a/GoogleSignIn/Tests/Unit/GIDAuthStateMigrationTest.m +++ b/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 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 @@ @implementation GIDAuthStateMigrationTest { 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 @@ - (void)setUp { _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 @@ - (void)tearDown { [_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 @@ - (void)testMigrateIfNeeded_NoPreviousMigration { 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 @@ - (void)testMigrateIfNeeded_HasPreviousMigration { 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 @@ - (void)testMigrateIfNeeded_KeychainFailure { 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 @@ - (void)testExtractAuthorization_HostedDomain { 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 diff --git a/README.md b/README.md index 55fb6d94..3376e1f4 100644 --- a/README.md +++ b/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 diff --git a/Samples/Swift/DaysUntilBirthday/README.md b/Samples/Swift/DaysUntilBirthday/README.md index e19bdf0a..9b49509c 100644 --- a/Samples/Swift/DaysUntilBirthday/README.md +++ b/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. diff --git a/Samples/Swift/DaysUntilBirthday/macOS/DaysUntilBirthdayOnMac.entitlements b/Samples/Swift/DaysUntilBirthday/macOS/DaysUntilBirthdayOnMac.entitlements index 625af03d..36f62aaf 100644 --- a/Samples/Swift/DaysUntilBirthday/macOS/DaysUntilBirthdayOnMac.entitlements +++ b/Samples/Swift/DaysUntilBirthday/macOS/DaysUntilBirthdayOnMac.entitlements @@ -2,6 +2,10 @@ + keychain-access-groups + + $(AppIdentifierPrefix)$(CFBundleIdentifier) + com.apple.security.app-sandbox com.apple.security.files.user-selected.read-only