Code refactoring to implement suggestions from the code review.

The fileURL for the current document does no longer have to be passed
in as a parameter but instead is retreived by accessing the windowControllers
document that hosts the MPPasswordInputController.

_touchIdHandleUnlockAttempt got renamed to _touchIdUpdateKeyForCurrentDocument
to better state its actual purpose and should no longer be called
on unsuccessfull unlock attempts. It also now removes stored keys if the
state of the TouchIdEnabled button has changed.

The key, that is derived from the document, is now the same whether it is
used to store it in the transient dictionary or the userdefaults
This commit is contained in:
Julius Zint
2021-02-21 11:27:41 +01:00
parent 88a5af995e
commit 197a4145e8
4 changed files with 81 additions and 50 deletions

View File

@@ -436,7 +436,7 @@ NSString *const MPDocumentGroupKey = @"MPDocumentGrou
} }
// just return yes regardless since we will display the sheet again if needed! // just return yes regardless since we will display the sheet again if needed!
return YES; return YES;
} forFile:nil]; }];
sheet.contentViewController = passwordInputController; sheet.contentViewController = passwordInputController;
[self.windowForSheet beginSheet:sheet completionHandler:^(NSModalResponse returnCode) { /* nothing to do, rest is done in other handler! */ }]; [self.windowForSheet beginSheet:sheet completionHandler:^(NSModalResponse returnCode) { /* nothing to do, rest is done in other handler! */ }];
} }

View File

@@ -325,16 +325,12 @@ typedef void (^MPPasswordChangedBlock)(BOOL didChangePassword);
self.passwordInputController = [[MPPasswordInputController alloc] init]; self.passwordInputController = [[MPPasswordInputController alloc] init];
} }
self.contentViewController = self.passwordInputController; self.contentViewController = self.passwordInputController;
NSURL* fileURL = nil;
if(self.document != nil) {
fileURL = [self.document fileURL];
}
[self.passwordInputController requestPasswordWithMessage:message cancelLabel:nil completionHandler:^BOOL(KPKCompositeKey* compositeKey, NSURL* keyURL, BOOL didCancel, NSError *__autoreleasing *error) { [self.passwordInputController requestPasswordWithMessage:message cancelLabel:nil completionHandler:^BOOL(KPKCompositeKey* compositeKey, NSURL* keyURL, BOOL didCancel, NSError *__autoreleasing *error) {
if(didCancel) { if(didCancel) {
return NO; return NO;
} }
return [((MPDocument *)self.document) unlockWithPassword:compositeKey keyFileURL:keyURL error:error ]; return [((MPDocument *)self.document) unlockWithPassword:compositeKey keyFileURL:keyURL error:error ];
} forFile:fileURL]; }];
} }
- (void)editPassword:(id)sender { - (void)editPassword:(id)sender {

View File

@@ -29,7 +29,7 @@
typedef BOOL (^passwordInputCompletionBlock)(KPKCompositeKey *key, NSURL* keyFileURL, BOOL didCancel, NSError *__autoreleasing*error); typedef BOOL (^passwordInputCompletionBlock)(KPKCompositeKey *key, NSURL* keyFileURL, BOOL didCancel, NSError *__autoreleasing*error);
- (void)requestPasswordWithMessage:(NSString *)message cancelLabel:(NSString *)cancelLabel completionHandler:(passwordInputCompletionBlock)completionHandler forFile:(NSURL*) fileURL; - (void)requestPasswordWithMessage:(NSString *)message cancelLabel:(NSString *)cancelLabel completionHandler:(passwordInputCompletionBlock)completionHandler;
@end @end

View File

@@ -53,7 +53,6 @@ static NSMutableDictionary* touchIDSecuredPasswords;
@property (copy) NSString *message; @property (copy) NSString *message;
@property (copy) NSString *cancelLabel; @property (copy) NSString *cancelLabel;
@property (copy) NSURL *databaseFileURL;
@property (assign) BOOL showPassword; @property (assign) BOOL showPassword;
@property (nonatomic, assign) BOOL enablePassword; @property (nonatomic, assign) BOOL enablePassword;
@@ -95,7 +94,7 @@ static NSMutableDictionary* touchIDSecuredPasswords;
if (@available(macOS 10.13.4, *)) { if (@available(macOS 10.13.4, *)) {
self.touchIdEnabled.hidden = false; self.touchIdEnabled.hidden = false;
self.touchIdEnabled.state = [NSUserDefaults.standardUserDefaults integerForKey:kMPSettingsKeyEntryTouchIdEnabled]; self.touchIdEnabled.state = [NSUserDefaults.standardUserDefaults integerForKey:kMPSettingsKeyEntryTouchIdEnabled];
[self _updateTouchIdTooltip]; [self _touchIdUpdateToolTip];
} }
[self _reset]; [self _reset];
} }
@@ -104,11 +103,10 @@ static NSMutableDictionary* touchIDSecuredPasswords;
return self.passwordTextField; return self.passwordTextField;
} }
- (void)requestPasswordWithMessage:(NSString *)message cancelLabel:(NSString *)cancelLabel completionHandler:(passwordInputCompletionBlock)completionHandler forFile:(NSURL*) fileURL{ - (void)requestPasswordWithMessage:(NSString *)message cancelLabel:(NSString *)cancelLabel completionHandler:(passwordInputCompletionBlock)completionHandler {
self.completionHandler = completionHandler; self.completionHandler = completionHandler;
self.message = message; self.message = message;
self.cancelLabel = cancelLabel; self.cancelLabel = cancelLabel;
self.databaseFileURL = fileURL;
[self _reset]; [self _reset];
} }
@@ -147,9 +145,17 @@ static NSMutableDictionary* touchIDSecuredPasswords;
KPKCompositeKey* compositeKey = [[KPKCompositeKey alloc] init]; KPKCompositeKey* compositeKey = [[KPKCompositeKey alloc] init];
[compositeKey addKey:passwordKey]; [compositeKey addKey:passwordKey];
[compositeKey addKey:fileKey]; [compositeKey addKey:fileKey];
/* After the completion handler finished we no longer have a windowController set */
NSString* documentKey = NULL;
bool documentKeyValid = [self _touchIdGetKeyForCurrentDocument:&documentKey];
BOOL result = self.completionHandler(compositeKey, keyURL, cancel, &error); BOOL result = self.completionHandler(compositeKey, keyURL, cancel, &error);
[self _touchIdHandleUnlockAttempt:compositeKey withResult:result]; if(result) {
if(cancel || result) { if(documentKeyValid) {
[self _touchIdUpdateKeyForCurrentDocument:compositeKey forDocumentKey:documentKey];
}
return;
}
if(cancel) {
return; return;
} }
[self _showError:error]; [self _showError:error];
@@ -159,18 +165,24 @@ static NSMutableDictionary* touchIDSecuredPasswords;
} }
} }
- (void) _touchIdHandleUnlockAttempt: (KPKCompositeKey*)compositeKey withResult:(bool)success { - (void) _touchIdUpdateKeyForCurrentDocument: (KPKCompositeKey*)compositeKey forDocumentKey: (NSString*) documentKey{
if(success && self.databaseFileURL && self.databaseFileURL.lastPathComponent) { NSData* encryptedKey = [self _touchIdEncryptCompositeKey:compositeKey];
NSData* encryptedKey = [self _touchIdEncryptCompositeKey:compositeKey]; if (self.touchIdEnabled.state == NSControlStateValueMixed) {
if(encryptedKey) { [NSUserDefaults.standardUserDefaults removeObjectForKey:documentKey];
if (self.touchIdEnabled.state == NSControlStateValueMixed) { if(encryptedKey != NULL) {
[touchIDSecuredPasswords setObject:encryptedKey forKey:self.databaseFileURL.lastPathComponent]; [touchIDSecuredPasswords setObject:encryptedKey forKey:documentKey];
}
else if(self.touchIdEnabled.state == NSControlStateValueOn) {
[NSUserDefaults.standardUserDefaults setObject:encryptedKey forKey:[self _userDefaultsKeyForEncryptedCompositeKey]];
}
} }
} }
else if(self.touchIdEnabled.state == NSControlStateValueOn) {
[touchIDSecuredPasswords removeObjectForKey:documentKey];
if(encryptedKey != NULL) {
[NSUserDefaults.standardUserDefaults setObject:encryptedKey forKey:documentKey];
}
}
else {
[NSUserDefaults.standardUserDefaults removeObjectForKey:documentKey];
[touchIDSecuredPasswords removeObjectForKey:documentKey];
}
} }
- (void) _touchIdCreateAndAddRSAKeyPair { - (void) _touchIdCreateAndAddRSAKeyPair {
@@ -260,7 +272,9 @@ static NSMutableDictionary* touchIDSecuredPasswords;
else { else {
NSLog(@"The key retreived from the Keychain is unable to encrypt data"); NSLog(@"The key retreived from the Keychain is unable to encrypt data");
} }
if (publicKey) { CFRelease(publicKey); } if (publicKey) {
CFRelease(publicKey);
}
return encryptedKey; return encryptedKey;
} }
@@ -305,47 +319,67 @@ static NSMutableDictionary* touchIDSecuredPasswords;
return result; return result;
} }
- (NSString*) _userDefaultsKeyForEncryptedCompositeKey { - (bool) _touchIdGetKeyForCurrentDocument: (NSString**) result {
NSString* result = [NSString stringWithFormat:kMPSettingsKeyEntryTouchIdDatabaseEncryptedKeyFormat, self.databaseFileURL.lastPathComponent]; *result = NULL;
return result; NSDocument* currentDocument = self.windowController.document;
if(currentDocument != NULL && currentDocument.fileURL != NULL && currentDocument.fileURL.lastPathComponent != NULL) {
*result = [NSString stringWithFormat:kMPSettingsKeyEntryTouchIdDatabaseEncryptedKeyFormat, currentDocument.fileURL.lastPathComponent];
return true;
}
return false;
} }
- (bool) _touchIdIsUnlockAvailable { - (bool) _touchIdIsUnlockAvailable {
bool result = false; NSData* unused = NULL;
if(self.databaseFileURL != nil && self.databaseFileURL.lastPathComponent != nil) bool encryptedKeyAvailableForDocument = [self _touchIdGetEncrypedKeyMaterial:&unused];
{ return encryptedKeyAvailableForDocument;
if ([touchIDSecuredPasswords valueForKey:self.databaseFileURL.lastPathComponent] != nil) { }
result = true;
} - (bool) _touchIdGetEncrypedKeyMaterial: (NSData**) result {
else if([NSUserDefaults.standardUserDefaults dataForKey:[self _userDefaultsKeyForEncryptedCompositeKey]] != nil) { NSString* documentKey = NULL;
result = true; *result = NULL;
} if(![self _touchIdGetKeyForCurrentDocument:&documentKey]) {
return false;
} }
return result; NSData* transientKey = [touchIDSecuredPasswords valueForKey:documentKey];
NSData* persistentKey =[NSUserDefaults.standardUserDefaults dataForKey:documentKey];
if(transientKey == NULL && persistentKey == NULL) {
return false;
}
if(transientKey == NULL || persistentKey == NULL) {
*result = transientKey == NULL ? persistentKey : transientKey;
return true;
}
if(self.touchIdEnabled.state == NSControlStateValueOn) {
*result = persistentKey;
return true;
}
*result = transientKey;
return true;
} }
- (IBAction)unlockWithTouchID:(id)sender { - (IBAction)unlockWithTouchID:(id)sender {
NSData* encryptedKey = [touchIDSecuredPasswords valueForKey:self.databaseFileURL.lastPathComponent]; NSData* encryptedKey = NULL;
if(!encryptedKey) { if(![self _touchIdGetEncrypedKeyMaterial:&encryptedKey]) {
encryptedKey = [NSUserDefaults.standardUserDefaults dataForKey:[self _userDefaultsKeyForEncryptedCompositeKey]]; [self.touchIdButton setEnabled:false];
return;
} }
KPKCompositeKey* compositeKey = [self _touchIdDecryptCompositeKey:encryptedKey]; KPKCompositeKey* compositeKey = [self _touchIdDecryptCompositeKey:encryptedKey];
if(compositeKey != nil) { if(compositeKey == NULL) {
NSError* error; [self.touchIdButton setEnabled:false];
self.completionHandler(compositeKey, nil, false, &error); return;
[self _showError:error];
}
else {
self.touchIdButton.hidden = true;
} }
NSError* error;
self.completionHandler(compositeKey, NULL, false, &error);
[self _showError:error];
} }
- (IBAction)touchIdEnabledChanged:(id)sender { - (IBAction)touchIdEnabledChanged:(id)sender {
[NSUserDefaults.standardUserDefaults setInteger: self.touchIdEnabled.state forKey:kMPSettingsKeyEntryTouchIdEnabled]; [NSUserDefaults.standardUserDefaults setInteger: self.touchIdEnabled.state forKey:kMPSettingsKeyEntryTouchIdEnabled];
[self _updateTouchIdTooltip]; [self _touchIdUpdateToolTip];
} }
- (void) _updateTouchIdTooltip { - (void) _touchIdUpdateToolTip {
if(self.touchIdEnabled.state == NSControlStateValueOn) { if(self.touchIdEnabled.state == NSControlStateValueOn) {
self.touchIdEnabled.toolTip = @"Unlocking via TouchID is enabled"; self.touchIdEnabled.toolTip = @"Unlocking via TouchID is enabled";
} }
@@ -373,6 +407,7 @@ static NSMutableDictionary* touchIDSecuredPasswords;
self.passwordTextField.stringValue = @""; self.passwordTextField.stringValue = @"";
self.messageInfoTextField.hidden = (nil == self.message); self.messageInfoTextField.hidden = (nil == self.message);
self.touchIdButton.hidden = ![self _touchIdIsUnlockAvailable]; self.touchIdButton.hidden = ![self _touchIdIsUnlockAvailable];
[self.touchIdButton setEnabled:true];
if(self.message) { if(self.message) {
self.messageInfoTextField.stringValue = self.message; self.messageInfoTextField.stringValue = self.message;