Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion FirebaseInstallations.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ Pod::Spec.new do |s|
s.framework = 'Security'
s.dependency 'FirebaseCore', '~> 6.6'
s.dependency 'PromisesObjC', '~> 1.2'
s.dependency 'GoogleUtilities/UserDefaults', '~> 6.5'
s.dependency 'GoogleUtilities/Environment', '~> 6.6'
s.dependency 'GoogleUtilities/UserDefaults', '~> 6.6'

preprocessor_definitions = 'FIRInstallations_LIB_VERSION=' + String(s.version)
if ENV['FIS_ALLOWS_INCOMPATIBLE_IID_VERSION'] && ENV['FIS_ALLOWS_INCOMPATIBLE_IID_VERSION'] == '1' then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@
#import "FBLPromises.h"
#endif

#import <GoogleUtilities/GULKeychainUtils.h>

#import "FIRInstallationsErrorUtil.h"
#import "FIRInstallationsKeychainUtils.h"

static NSString *const kFIRInstallationsIIDTokenKeychainId = @"com.google.iid-tokens";

Expand Down Expand Up @@ -118,7 +119,7 @@ - (instancetype)initWithGCMSenderID:(NSString *)GCMSenderID {

NSMutableDictionary *keychainQuery = [self IIDDefaultTokenDataKeychainQuery];
NSError *error;
NSData *data = [FIRInstallationsKeychainUtils getItemWithQuery:keychainQuery error:&error];
NSData *data = [GULKeychainUtils getItemWithQuery:keychainQuery error:&error];

if (data) {
[resultPromise fulfill:data];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
#import "FBLPromises.h"
#endif

#import <GoogleUtilities/GULKeychainStorage.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: alphabetically below FirebaseCore


#import <FirebaseCore/FIRAppInternal.h>

#import "FIRInstallationsAPIService.h"
Expand All @@ -32,7 +34,6 @@
#import "FIRInstallationsLogger.h"
#import "FIRInstallationsSingleOperationPromiseCache.h"
#import "FIRInstallationsStore.h"
#import "FIRSecureStorage.h"

#import "FIRInstallationsHTTPError.h"
#import "FIRInstallationsStoredAuthToken.h"
Expand Down Expand Up @@ -72,7 +73,7 @@ - (instancetype)initWithGoogleAppID:(NSString *)appID
projectID:(NSString *)projectID
GCMSenderID:(NSString *)GCMSenderID
accessGroup:(NSString *)accessGroup {
FIRSecureStorage *secureStorage = [[FIRSecureStorage alloc] init];
GULKeychainStorage *secureStorage = [[GULKeychainStorage alloc] init];
FIRInstallationsStore *installationsStore =
[[FIRInstallationsStore alloc] initWithSecureStorage:secureStorage accessGroup:accessGroup];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

@class FBLPromise<ValueType>;
@class FIRInstallationsItem;
@class FIRSecureStorage;
@class GULKeychainStorage;

NS_ASSUME_NONNULL_BEGIN

Expand All @@ -33,7 +33,7 @@ extern NSString *const kFIRInstallationsStoreUserDefaultsID;
* @param storage The secure storage to save installations data.
* @param accessGroup The Keychain Access Group to store and request the installations data.
*/
- (instancetype)initWithSecureStorage:(FIRSecureStorage *)storage
- (instancetype)initWithSecureStorage:(GULKeychainStorage *)storage
accessGroup:(nullable NSString *)accessGroup;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,24 @@
#import "FBLPromises.h"
#endif

#import <GoogleUtilities/GULKeychainStorage.h>

#import "FIRInstallationsErrorUtil.h"
#import "FIRInstallationsItem.h"
#import "FIRInstallationsStoredItem.h"
#import "FIRSecureStorage.h"

NSString *const kFIRInstallationsStoreUserDefaultsID = @"com.firebase.FIRInstallations";

@interface FIRInstallationsStore ()
@property(nonatomic, readonly) FIRSecureStorage *secureStorage;
@property(nonatomic, readonly) GULKeychainStorage *secureStorage;
@property(nonatomic, readonly, nullable) NSString *accessGroup;
@property(nonatomic, readonly) dispatch_queue_t queue;
@property(nonatomic, readonly) GULUserDefaults *userDefaults;
@end

@implementation FIRInstallationsStore

- (instancetype)initWithSecureStorage:(FIRSecureStorage *)storage
- (instancetype)initWithSecureStorage:(GULKeychainStorage *)storage
accessGroup:(NSString *)accessGroup {
self = [super init];
if (self) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,15 @@

#import <OCMock/OCMock.h>

#import <GoogleUtilities/GULKeychainStorage.h>
#import <GoogleUtilities/GULUserDefaults.h>

#import "FBLPromise+Testing.h"
#import "FIRInstallationsErrorUtil.h"
#import "FIRInstallationsItem+Tests.h"
#import "FIRInstallationsItem.h"
#import "FIRInstallationsStore.h"
#import "FIRInstallationsStoredItem.h"
#import "FIRSecureStorage.h"

@interface FIRInstallationsStoreTests : XCTestCase
@property(nonatomic) NSString *accessGroup;
Expand All @@ -38,7 +39,7 @@ @implementation FIRInstallationsStoreTests

- (void)setUp {
self.accessGroup = @"accessGroup";
self.mockSecureStorage = OCMClassMock([FIRSecureStorage class]);
self.mockSecureStorage = OCMClassMock([GULKeychainStorage class]);
self.store = [[FIRInstallationsStore alloc] initWithSecureStorage:self.mockSecureStorage
accessGroup:self.accessGroup];

Expand Down
4 changes: 3 additions & 1 deletion GoogleUtilities.podspec
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Pod::Spec.new do |s|
s.name = 'GoogleUtilities'
s.version = '6.5.2'
s.version = '6.6.0'
s.summary = 'Google Utilities for iOS (plus community support for macOS and tvOS)'

s.description = <<-DESC
Expand Down Expand Up @@ -34,6 +34,8 @@ other Google CocoaPods. They're not intended for direct public usage.
es.source_files = 'GoogleUtilities/Environment/**/*.[mh]'
es.public_header_files = 'GoogleUtilities/Environment/**/*.h'
es.private_header_files = 'GoogleUtilities/Environment/**/*.h'

es.dependency 'PromisesObjC', '~> 1.2'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate from this review - but are we comfortable allowing minor version updates of Promises. It might be safer to lock to patch version updates ...

end

s.subspec 'Logger' do |ls|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
NS_ASSUME_NONNULL_BEGIN

/// The class provides a convenient abstraction on top of the iOS Keychain API to save data.
@interface FIRSecureStorage : NSObject
@interface GULKeychainStorage : NSObject

/**
* Get an object by key.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@

NS_ASSUME_NONNULL_BEGIN

FOUNDATION_EXPORT NSString *const kGULKeychainUtilsErrorDomain;

/// Helper functions to access Keychain.
@interface FIRInstallationsKeychainUtils : NSObject
@interface GULKeychainUtils : NSObject

// TODO: Add API docs.
+ (nullable NSData *)getItemWithQuery:(NSDictionary *)query
error:(NSError *_Nullable *_Nullable)outError;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

#import "FIRSecureStorage.h"
#import "GULKeychainStorage.h"
#import <Security/Security.h>

#if __has_include(<FBLPromises/FBLPromises.h>)
Expand All @@ -23,32 +23,33 @@
#import "FBLPromises.h"
#endif

#import "FIRInstallationsErrorUtil.h"
#import "FIRInstallationsKeychainUtils.h"
#import <GoogleUtilities/GULSecureCoding.h>

@interface FIRSecureStorage ()
#import "GULKeychainUtils.h"

@interface GULKeychainStorage ()
@property(nonatomic, readonly) dispatch_queue_t keychainQueue;
@property(nonatomic, readonly) dispatch_queue_t inMemoryCacheQueue;
@property(nonatomic, readonly) NSString *service;
@property(nonatomic, readonly) NSCache<NSString *, id<NSSecureCoding>> *inMemoryCache;
@end

@implementation FIRSecureStorage
@implementation GULKeychainStorage

- (instancetype)init {
NSCache *cache = [[NSCache alloc] init];
// Cache up to 5 installations.
cache.countLimit = 5;
return [self initWithService:@"com.firebase.FIRInstallations.installations" cache:cache];
return [self initWithService:@"com.gul.KeychainStorage" cache:cache];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the location has changed, how do we ensure the old keychain data is not wiped when update to this version?
And do we care about backward compatibility?

Copy link
Contributor Author

@maksymmalyhin maksymmalyhin Apr 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Let me make sure the service name is always set by client, so Installations will keep using the same service as before.

}

- (instancetype)initWithService:(NSString *)service cache:(NSCache *)cache {
self = [super init];
if (self) {
_keychainQueue = dispatch_queue_create(
"com.firebase.FIRInstallations.FIRSecureStorage.Keychain", DISPATCH_QUEUE_SERIAL);
_inMemoryCacheQueue = dispatch_queue_create(
"com.firebase.FIRInstallations.FIRSecureStorage.InMemoryCache", DISPATCH_QUEUE_SERIAL);
_keychainQueue =
dispatch_queue_create("com.gul.KeychainStorage.Keychain", DISPATCH_QUEUE_SERIAL);
_inMemoryCacheQueue =
dispatch_queue_create("com.gul.KeychainStorage.InMemoryCache", DISPATCH_QUEUE_SERIAL);
_service = [service copy];
_inMemoryCache = cache;
}
Expand Down Expand Up @@ -91,12 +92,12 @@ - (instancetype)initWithService:(NSString *)service cache:(NSCache *)cache {
// Then store the object to the keychain.
NSDictionary *query = [self keychainQueryWithKey:key accessGroup:accessGroup];
NSError *error;
NSData *encodedObject = [self archiveDataForObject:object error:&error];
NSData *encodedObject = [GULSecureCoding archivedDataWithRootObject:object error:&error];
if (!encodedObject) {
return error;
}

if (![FIRInstallationsKeychainUtils setItem:encodedObject withQuery:query error:&error]) {
if (![GULKeychainUtils setItem:encodedObject withQuery:query error:&error]) {
return error;
}

Expand All @@ -115,7 +116,7 @@ - (instancetype)initWithService:(NSString *)service cache:(NSCache *)cache {
NSDictionary *query = [self keychainQueryWithKey:key accessGroup:accessGroup];

NSError *error;
if (![FIRInstallationsKeychainUtils removeItemWithQuery:query error:&error]) {
if (![GULKeychainUtils removeItemWithQuery:query error:&error]) {
return error;
}

Expand All @@ -129,29 +130,28 @@ - (instancetype)initWithService:(NSString *)service cache:(NSCache *)cache {
objectClass:(Class)objectClass
accessGroup:(nullable NSString *)accessGroup {
// Look for the object in the keychain.
return [FBLPromise onQueue:self.keychainQueue
do:^id {
NSDictionary *query = [self keychainQueryWithKey:key
accessGroup:accessGroup];
NSError *error;
NSData *encodedObject =
[FIRInstallationsKeychainUtils getItemWithQuery:query error:&error];

if (error) {
return error;
}
if (!encodedObject) {
return nil;
}
id object = [self unarchivedObjectOfClass:objectClass
fromData:encodedObject
error:&error];
if (error) {
return error;
}

return object;
}]
return [FBLPromise
onQueue:self.keychainQueue
do:^id {
NSDictionary *query = [self keychainQueryWithKey:key accessGroup:accessGroup];
NSError *error;
NSData *encodedObject = [GULKeychainUtils getItemWithQuery:query error:&error];

if (error) {
return error;
}
if (!encodedObject) {
return nil;
}
id object = [GULSecureCoding unarchivedObjectOfClass:objectClass
fromData:encodedObject
error:&error];
if (error) {
return error;
}

return object;
}]
.thenOn(self.inMemoryCacheQueue,
^id<NSSecureCoding> _Nullable(id<NSSecureCoding> _Nullable object) {
// Save object to the in-memory cache if exists and return the object.
Expand Down Expand Up @@ -190,66 +190,4 @@ - (void)resetInMemoryCache {
return query;
}

- (nullable NSData *)archiveDataForObject:(id<NSSecureCoding>)object error:(NSError **)outError {
NSData *archiveData;
if (@available(macOS 10.13, iOS 11.0, tvOS 11.0, *)) {
NSError *error;
archiveData = [NSKeyedArchiver archivedDataWithRootObject:object
requiringSecureCoding:YES
error:&error];
if (error && outError) {
*outError = [FIRInstallationsErrorUtil keyedArchiverErrorWithError:error];
}
} else {
@try {
NSMutableData *data = [NSMutableData data];
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
NSKeyedArchiver *archiver = [[NSKeyedArchiver alloc] initForWritingWithMutableData:data];
#pragma clang diagnostic pop
archiver.requiresSecureCoding = YES;

[archiver encodeObject:object forKey:NSKeyedArchiveRootObjectKey];
[archiver finishEncoding];

archiveData = [data copy];
} @catch (NSException *exception) {
if (outError) {
*outError = [FIRInstallationsErrorUtil keyedArchiverErrorWithException:exception];
}
}
}

return archiveData;
}

- (nullable id)unarchivedObjectOfClass:(Class)class
fromData:(NSData *)data
error:(NSError **)outError {
id object;
if (@available(macOS 10.13, iOS 11.0, tvOS 11.0, *)) {
NSError *error;
object = [NSKeyedUnarchiver unarchivedObjectOfClass:class fromData:data error:&error];
if (error && outError) {
*outError = [FIRInstallationsErrorUtil keyedArchiverErrorWithError:error];
}
} else {
@try {
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
NSKeyedUnarchiver *unarchiver = [[NSKeyedUnarchiver alloc] initForReadingWithData:data];
#pragma clang diagnostic pop
unarchiver.requiresSecureCoding = YES;

object = [unarchiver decodeObjectOfClass:class forKey:NSKeyedArchiveRootObjectKey];
} @catch (NSException *exception) {
if (outError) {
*outError = [FIRInstallationsErrorUtil keyedArchiverErrorWithException:exception];
}
}
}

return object;
}

@end
Loading