fix(AWSCore): Add sync control to avoid crash during concurrent credential requests (#4527)

* Add sync control to avoid crash during concurrent credential requests

* Update CHANGELOG.md

---------

Co-authored-by: Jithin Roy <[email protected]>
Co-authored-by: Harshdeep Singh <[email protected]>

Author: 3 people

AWSCore/Authentication/AWSCredentialsProvider.h

@@ -176,7 +176,7 @@ typedef NS_ENUM(NSInteger, AWSCognitoCredentialsProviderErrorType) {
 /**
  The identity id associated with this provider. This value will be fetched from the keychain at startup. If you do not want to reuse the existing identity id, you must call the clearKeychain method.
  */
-@property (nonatomic, strong, readonly, nullable) NSString *identityId;
+@property (atomic, strong, readonly, nullable) NSString *identityId;
 
 /**
  The identity pool id associated with this provider. Also used to create a namedspaced keychain area to store identity id and credentials.

AWSCore/Authentication/AWSCredentialsProvider.m

@@ -194,7 +194,7 @@ @interface AWSWebIdentityCredentialsProvider()
 
 @property (nonatomic, strong) AWSSTS *sts;
 @property (nonatomic, strong) AWSUICKeyChainStore *keychain;
-@property (nonatomic, strong) AWSCredentials *internalCredentials;
+@property (atomic, strong) AWSCredentials *internalCredentials;
 
 @end
 
@@ -267,22 +267,26 @@ - (void)invalidateCachedTemporaryCredentials {
 #pragma mark -
 
 - (AWSCredentials *)internalCredentials {
-    if (! _internalCredentials) {
-        _internalCredentials = [[AWSCredentials alloc] initFromKeychain:self.keychain];
+    @synchronized (self) {
+        if (! _internalCredentials) {
+            _internalCredentials = [[AWSCredentials alloc] initFromKeychain:self.keychain];
+        }
+        return _internalCredentials;
     }
-    return _internalCredentials;
 }
 
 - (void)setInternalCredentials:(AWSCredentials *)internalCredentials {
-    _internalCredentials = internalCredentials;
-
-    self.keychain[AWSCredentialsProviderKeychainAccessKeyId] = internalCredentials.accessKey;
-    self.keychain[AWSCredentialsProviderKeychainSecretAccessKey] = internalCredentials.secretKey;
-    self.keychain[AWSCredentialsProviderKeychainSessionToken] = internalCredentials.sessionKey;
-    if (internalCredentials.expiration) {
-        self.keychain[AWSCredentialsProviderKeychainExpiration] = [NSString stringWithFormat:@"%f", [internalCredentials.expiration timeIntervalSince1970]];
-    } else {
-        self.keychain[AWSCredentialsProviderKeychainExpiration] = nil;
+    @synchronized (self) {
+        _internalCredentials = internalCredentials;
+
+        self.keychain[AWSCredentialsProviderKeychainAccessKeyId] = internalCredentials.accessKey;
+        self.keychain[AWSCredentialsProviderKeychainSecretAccessKey] = internalCredentials.secretKey;
+        self.keychain[AWSCredentialsProviderKeychainSessionToken] = internalCredentials.sessionKey;
+        if (internalCredentials.expiration) {
+            self.keychain[AWSCredentialsProviderKeychainExpiration] = [NSString stringWithFormat:@"%f", [internalCredentials.expiration timeIntervalSince1970]];
+        } else {
+            self.keychain[AWSCredentialsProviderKeychainExpiration] = nil;
+        }
     }
 }
 
@@ -298,9 +302,9 @@ @interface AWSCognitoCredentialsProvider()
 @property (nonatomic, strong) AWSExecutor *refreshExecutor;
 @property (nonatomic, strong) dispatch_semaphore_t semaphore;
 @property (atomic, assign) BOOL useEnhancedFlow;
-@property (nonatomic, strong) AWSCredentials *internalCredentials;
+@property (atomic, strong) AWSCredentials *internalCredentials;
 @property (atomic, assign, getter=isRefreshingCredentials) BOOL refreshingCredentials;
-@property (nonatomic, strong) NSDictionary<NSString *, NSString *> *cachedLogins;
+@property (atomic, strong) NSDictionary<NSString *, NSString *> *cachedLogins;
 // This is a temporary solution to bypass the requirement of protocol check for `AWSIdentityProviderManager`.
 @property (nonatomic, strong) NSString *customRoleArnOverride;
 
@@ -659,7 +663,8 @@ - (void)setUpWithRegionType:(AWSRegionType)regionType
             // 2. The cached credentials is nil.
             // 3. The credentials expire within 10 minutes.
             credentials = self.internalCredentials.copy;
-            if ((!self.cachedLogins || [self.cachedLogins isEqualToDictionary:logins])
+            NSDictionary<NSString *, NSString *> *cachedLogins = self.cachedLogins;
+            if ((!cachedLogins || [cachedLogins isEqualToDictionary:logins])
                 && credentials
                 && credentials.isValid) {
                 return [AWSTask taskWithResult:credentials];
@@ -679,7 +684,8 @@ - (void)setUpWithRegionType:(AWSRegionType)regionType
                 return [AWSTask cancelledTask];
             }
             credentials = self.internalCredentials.copy;
-            if ((!self.cachedLogins || [self.cachedLogins isEqualToDictionary:logins])
+            cachedLogins = self.cachedLogins;
+            if ((!cachedLogins || [cachedLogins isEqualToDictionary:logins])
                 && credentials
                 && credentials.isValid) {
                 return [AWSTask taskWithResult:credentials];
@@ -806,30 +812,39 @@ - (NSString *)identityId {
     if (identityId) {
         return identityId;
     }
-    return [self.keychain stringForKey:AWSCredentialsProviderKeychainIdentityId];
+
+    @synchronized (self) {
+        return [self.keychain stringForKey:AWSCredentialsProviderKeychainIdentityId];
+    }
 }
 
 - (void)setIdentityId:(NSString *)identityId {
-    self.keychain[AWSCredentialsProviderKeychainIdentityId] = identityId;
+    @synchronized (self) {
+        self.keychain[AWSCredentialsProviderKeychainIdentityId] = identityId;
+    }
 }
 
 - (AWSCredentials *)internalCredentials {
-    if (! _internalCredentials) {
-        _internalCredentials = [[AWSCredentials alloc] initFromKeychain:self.keychain];
+    @synchronized (self) {
+        if (!_internalCredentials) {
+            _internalCredentials = [[AWSCredentials alloc] initFromKeychain:self.keychain];
+        }
+        return _internalCredentials;
     }
-    return _internalCredentials;
 }
 
 - (void)setInternalCredentials:(AWSCredentials *)internalCredentials {
-    _internalCredentials = internalCredentials;
-
-    self.keychain[AWSCredentialsProviderKeychainAccessKeyId] = internalCredentials.accessKey;
-    self.keychain[AWSCredentialsProviderKeychainSecretAccessKey] = internalCredentials.secretKey;
-    self.keychain[AWSCredentialsProviderKeychainSessionToken] = internalCredentials.sessionKey;
-    if (internalCredentials.expiration) {
-        self.keychain[AWSCredentialsProviderKeychainExpiration] = [NSString stringWithFormat:@"%f", [internalCredentials.expiration timeIntervalSince1970]];
-    } else {
-        self.keychain[AWSCredentialsProviderKeychainExpiration] = nil;
+    @synchronized (self) {
+        _internalCredentials = internalCredentials;
+
+        self.keychain[AWSCredentialsProviderKeychainAccessKeyId] = internalCredentials.accessKey;
+        self.keychain[AWSCredentialsProviderKeychainSecretAccessKey] = internalCredentials.secretKey;
+        self.keychain[AWSCredentialsProviderKeychainSessionToken] = internalCredentials.sessionKey;
+        if (internalCredentials.expiration) {
+            self.keychain[AWSCredentialsProviderKeychainExpiration] = [NSString stringWithFormat:@"%f", [internalCredentials.expiration timeIntervalSince1970]];
+        } else {
+            self.keychain[AWSCredentialsProviderKeychainExpiration] = nil;
+        }
     }
 }
 

AWSCore/Authentication/AWSIdentityProvider.h

@@ -87,7 +87,7 @@ typedef NS_ENUM(NSInteger, AWSCognitoCredentialsProviderHelperErrorType) {
 /**
  The identity id as determined by the Amazon Cognito service
  */
-@property (nonatomic, strong, nullable) NSString *identityId;
+@property (atomic, strong, nullable) NSString *identityId;
 
 /**
  */
@@ -123,7 +123,7 @@ typedef NS_ENUM(NSInteger, AWSCognitoCredentialsProviderHelperErrorType) {
 /**
  The identity id as determined by the Amazon Cognito service
  */
-@property (nonatomic, strong, nullable) NSString *identityId;
+@property (atomic, strong, nullable) NSString *identityId;
 
 /**
  The identity provider manager that asynchronously returns `logins`.

AWSCore/Authentication/AWSIdentityProvider.m

@@ -41,7 +41,7 @@ @interface AWSAbstractCognitoCredentialsProviderHelper()
 
 @property (nonatomic, strong) id<AWSIdentityProviderManager> identityProviderManager;
 @property (nonatomic, strong) NSString *identityPoolId;
-@property (nonatomic, strong) NSDictionary *cachedLogins;
+@property (atomic, strong) NSDictionary *cachedLogins;
 
 @end
 
@@ -53,6 +53,8 @@ - (instancetype)initWithConfiguration:(AWSServiceConfiguration *)configuration;
 
 @implementation AWSAbstractCognitoCredentialsProviderHelper
 
+@synthesize identityId = _identityId;
+
 #pragma mark - AWSIdentityProvider
 
 // Sub classes should override this.
@@ -88,11 +90,19 @@ - (BOOL)isAuthenticated {
     return [self.cachedLogins count] > 0;
 }
 
+- (NSString *)identityId {
+    @synchronized (self) {
+        return _identityId;
+    }
+}
+
 - (void)setIdentityId:(NSString *)identityId {
-    if (identityId && ![identityId isEqualToString:_identityId]) {
-        [self postIdentityIdChangedNotification:identityId];
+    @synchronized (self) {
+        if (identityId && ![identityId isEqualToString:_identityId]) {
+            [self postIdentityIdChangedNotification:identityId];
+        }
+        _identityId = identityId;
     }
-    _identityId = identityId;
 }
 
 - (void)postIdentityIdChangedNotification:(NSString *)newId {
@@ -272,10 +282,10 @@ - (NSString *)identityProviderName {
     if (self.identityProviderManager && self.useEnhancedFlow) {
         self.cachedLogins = nil;
         return [[self getIdentityId] continueWithSuccessBlock:^id _Nullable(AWSTask<NSString *> * _Nonnull task) {
-            if(self.cachedLogins){
-                return [AWSTask taskWithResult:self.cachedLogins];
-            }
-            else {
+            NSDictionary *cachedLogins = self.cachedLogins;
+            if (cachedLogins) {
+                return [AWSTask taskWithResult:cachedLogins];
+            } else {
                 return [self.identityProviderManager logins];
             }
         }];

CHANGELOG.md

@@ -1,5 +1,12 @@
 # AWS Mobile SDK for iOS CHANGELOG
 
+## 2.30.4
+
+### Bug Fixes
+
+- **AWSCore**
+    - Add sync control to avoid crash during concurrent credential requests
+
 ## 2.30.3
 
 ### Bug Fixes