Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Persistent Dictionary: Secure Coding Support #604

Merged
merged 6 commits into from Nov 6, 2020

Conversation

jleandroperez
Copy link
Contributor

@jleandroperez jleandroperez commented Nov 5, 2020

Details:

In this PR we're patching up SPPersistentMutableDictionary so that it supports the new Secure Coding API.

@eshurakov May I bug you with this one as well?
Thank you!!

Closes #603

Note:

The flag requiresSecureCoding (NSKeyedArchiver / NSKeyedUnarchiver), as per the documentation, will enforce that persisted / restored classes conform to NSSecureCoding.

However, it does not allow us to turn On / Off encryption. For such reason, a migration is not necessary.

If you set the receiver to require secure coding, it will throw an exception if you attempt to unarchive a class which does not conform to NSSecureCoding.

Plus: A new Unit Test has been added to verify that "Unsecure Archives" can be opened when this flag is On.

Testing:

@jleandroperez jleandroperez self-assigned this Nov 5, 2020
@jleandroperez jleandroperez added this to the v1.1 milestone Nov 5, 2020
Comment on lines +482 to 490
- (BOOL)canStoreObject:(id)anObject {
for (Class supportedClass in self.supportedObjectTypes) {
if ([anObject isKindOfClass:supportedClass]) {
return YES;
}
}

return NO;
}

Choose a reason for hiding this comment

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

Do we need to check recursively in case of NSArray / NSDictionary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reasoning behind that assertion is: ObjC doesn't really have Generics, but "Lightweight Generics".

You can set a __covariant type in the header file, but it's only for Swift bridging. So I tried to do something that, at least, would give you a warning.

In our specific use case, all of the dictionaries contain either Strings or Numbers (everything is generated by a single API), so no need to validate further).

Plus the serialization invocation itself would blow up, if you pass along a non NSSecureCoding compliant class.

Thank you for bringing that up!!

Comment on lines 28 to 31
/// Indicates if the stored `Supported Object Types` should be required to conform to NSCoding. Defaults to YES
///
@property (nonatomic, assign, readwrite) BOOL requiringSecureCoding;

Choose a reason for hiding this comment

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

Is requiringSecureCoding only used for unit tests? Maybe it's good to mention it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

YES! Only for testing purposes, good call!!

@jleandroperez
Copy link
Contributor Author

Thank you @eshurakov !!

@jleandroperez jleandroperez merged commit 21aeee6 into develop Nov 6, 2020
@jleandroperez jleandroperez deleted the issue/603-secure-coding branch November 6, 2020 14:42
@jleandroperez jleandroperez mentioned this pull request Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warning: NSKeyedUnarchiveFromData
2 participants