Remove keychain entries if fresh install (#567)

Author: mdmathias

GoogleSignIn/Sources/GIDSignIn.m

@@ -120,8 +120,7 @@
 // Error string for user cancelations.
 static NSString *const kUserCanceledError = @"The user canceled the sign-in flow.";
 
-// User preference key to detect fresh install of the app.
-static NSString *const kAppHasRunBeforeKey = @"GID_AppHasRunBefore";
+NSString *const kAppHasRunBeforeKey = @"GID_AppHasRunBefore";
 
 // Maximum retry interval in seconds for the fetcher.
 static const NSTimeInterval kFetcherMaxRetryInterval = 15.0;
@@ -672,6 +671,11 @@ - (instancetype)initWithKeychainStore:(GTMKeychainStore *)keychainStore
 
     // Check to see if the 3P app is being run for the first time after a fresh install.
     BOOL isFreshInstall = [self isFreshInstall];
+    
+    // If this is a fresh install, ensure that any pre-existing keychain data is purged.
+    if (isFreshInstall) {
+      [self removeAllKeychainEntries];
+    }
 
     NSString *authorizationEnpointURL = [NSString stringWithFormat:kAuthorizationURLTemplate,
                                          [GIDSignInPreferences googleAuthorizationServer]];

GoogleSignIn/Sources/GIDSignIn_Private.h

@@ -32,6 +32,9 @@ NS_ASSUME_NONNULL_BEGIN
 @class GIDAppCheck;
 @class GIDAuthStateMigration;
 
+/// User preference key to detect fresh install of the app.
+extern NSString *const kAppHasRunBeforeKey;
+
 /// Represents a completion block that takes a `GIDSignInResult` on success or an error if the
 /// operation was unsuccessful.
 typedef void (^GIDSignInCompletion)(GIDSignInResult *_Nullable signInResult,

GoogleSignIn/Tests/Unit/GIDSignInTest.m

@@ -120,8 +120,6 @@
      @"com.google.UnitTests:///emmcallback?action=unrecognized";
 static NSString * const kDevicePolicyAppBundleID = @"com.google.DevicePolicy";
 
-static NSString * const kAppHasRunBeforeKey = @"GPP_AppHasRunBefore";
-
 static NSString * const kFingerprintKeychainName = @"fingerprint";
 static NSString * const kVerifierKeychainName = @"verifier";
 static NSString * const kVerifierKey = @"verifier";
@@ -1212,6 +1210,19 @@ - (void)testNotHandleWrongPath {
   XCTAssertFalse(_completionCalled, @"should not call delegate");
 }
 
+#pragma mark - Test Fresh Install
+
+- (void)testFreshInstall_removesKeychainEntries {
+  // Simulate that the app has been deleted and user defaults removed.
+  [NSUserDefaults.standardUserDefaults removeObjectForKey:kAppHasRunBeforeKey];
+  // Initialization should check `isFreshInstall`.
+  GIDSignIn *signIn = [[GIDSignIn alloc] initWithKeychainStore:_keychainStore
+                                     authStateMigrationService:_authStateMigrationService];
+  // If `isFreshInstall`, keychain entries should be removed.
+  XCTAssertNotNil(signIn);
+  XCTAssertTrue(self->_keychainRemoved);
+}
+
 #pragma mark - Tests - disconnectWithCallback:
 
 // Verifies disconnect calls callback with no errors if access token is present.

README.md

@@ -48,14 +48,15 @@ Google Sign-In allows your users to sign-in to your native macOS app using their
 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 credentials via the Keychain on macOS, you will need to add
-`$(AppIdentifierPrefix)$(CFBundleIdentifier)` to its keychain access group.
+`$(AppIdentifierPrefix)$(CFBundleIdentifier)` as the first item in 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 credentials via the Keychain on macOS, you will need to add
-`$(AppIdentifierPrefix)$(CFBundleIdentifier)` to its keychain access group.
+`$(AppIdentifierPrefix)$(CFBundleIdentifier)` as the first item in the keychain
+access group.
 
 ## Using the Google Sign-In Button
 
@@ -107,3 +108,19 @@ let signInButton = GoogleSignInButton {
 }
 let hostedButton = NSHostingView(rootView: signInButton)
 ```
+
+## A Note on iOS Keychain Access Groups
+
+GSI uses your default (first listed) keychain access group. If you don't add a
+custom keychain access group, the default keychain access group is provided by
+Xcode and looks like `$(AppIdentifierPrefix)$(CFBundleIdentifier)`.
+
+GSI [removes keychain items upon fresh install](https://github.com/google/GoogleSignIn-iOS/pull/567)
+to ensure that stale credentials from previous installs of your app are not
+mistakenly used. If your app uses a shared access group by default this may
+lead to new installs of apps sharing the same keychain access group to remove
+keychain credentials for apps already installed. 
+
+To prevent unintentional credential removal, you can explicitly list the
+typical default access group (or whatever you prefer so long as it is not
+shared) in your list first. GSI, will then use that default access group.