Update Common.m

[howiekaye]

In ValidateLoginKeychainPassword,

• if SecKeychainCopyDefault fails (ie, there is no defaultKeychain), the uninitialized defaultKeychain gets released (resulting in a crash).
• if SecKeychainCopyDefault fails, validation should fail, not succeed.

[russellhancox]

I disagree that validation should fail if a default keychain is not returned, it suggests the user doesn't yet have a keychain or has deleted it and either way means KeychainMinder can't do anything for them.

[howiekaye]

But, the validation did not succeed, there was still an error, just not an authfailed error. Maybe a BOOL is too limited as a return type.

On Wed, Feb 10, 2016 at 11:04 AM, Russell Hancox notifications@github.com wrote:

I disagree that validation should fail if a default keychain is not returned, it suggests the user doesn't yet have a keychain or has deleted it and either way means KeychainMinder can't do anything for them.

— Reply to this email directly or view it on GitHub https://github.com/google/macops-keychainminder/pull/6#issuecomment-182447516 .

[russellhancox]

The rest of the code doesn't need to know about any scenario other than "the keychain exists and it's password does not match the users' current password".

On Feb 10, 2016 11:16 AM, "Howie Kaye" notifications@github.com wrote:

But, the validation did not succeed, there was still an error, just not an authfailed error. Maybe a BOOL is too limited as a return type.

On Wed, Feb 10, 2016 at 11:04 AM, Russell Hancox <notifications@github.com

wrote:

I disagree that validation should fail if a default keychain is not returned, it suggests the user doesn't yet have a keychain or has deleted it and either way means KeychainMinder can't do anything for them.

— Reply to this email directly or view it on GitHub < https://github.com/google/macops-keychainminder/pull/6#issuecomment-182447516

.

— Reply to this email directly or view it on GitHub.

[howiekaye]

At the end of the app's operation, shouldn't the user have a usable keychain? If there is none, they still have problems. No keychain is closer to the "I forgot my password, so delete the keychain and recreate it for me" case.

On Wed, Feb 10, 2016 at 11:21 AM, Russell Hancox notifications@github.com wrote:

The rest of the code doesn't need to know about any scenario other than "the keychain exists and it's password does not match the users' current password".

On Feb 10, 2016 11:16 AM, "Howie Kaye" notifications@github.com wrote:

But, the validation did not succeed, there was still an error, just not an authfailed error. Maybe a BOOL is too limited as a return type.

On Wed, Feb 10, 2016 at 11:04 AM, Russell Hancox < notifications@github.com

wrote:

I disagree that validation should fail if a default keychain is not returned, it suggests the user doesn't yet have a keychain or has deleted it and either way means KeychainMinder can't do anything for them.

— Reply to this email directly or view it on GitHub <

https://github.com/google/macops-keychainminder/pull/6#issuecomment-182447516

.

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub https://github.com/google/macops-keychainminder/pull/6#issuecomment-182457644 .

[russellhancox]

No, the OS will handle creating a new keychain when it needs to. This function is used both in the app and the plugin, if we start failing validation when no keychain exists we'll popup after the OS has already fixed the problem. On Feb 10, 2016 11:33 AM, "Howie Kaye" notifications@github.com wrote:

At the end of the app's operation, shouldn't the user have a usable keychain? If there is none, they still have problems. No keychain is closer to the "I forgot my password, so delete the keychain and recreate it for me" case.

On Wed, Feb 10, 2016 at 11:21 AM, Russell Hancox <notifications@github.com

wrote:

The rest of the code doesn't need to know about any scenario other than "the keychain exists and it's password does not match the users' current password".

On Feb 10, 2016 11:16 AM, "Howie Kaye" notifications@github.com wrote:

But, the validation did not succeed, there was still an error, just not an authfailed error. Maybe a BOOL is too limited as a return type.

On Wed, Feb 10, 2016 at 11:04 AM, Russell Hancox < notifications@github.com

wrote:

I disagree that validation should fail if a default keychain is not returned, it suggests the user doesn't yet have a keychain or has deleted it and either way means KeychainMinder can't do anything for them.

— Reply to this email directly or view it on GitHub <

https://github.com/google/macops-keychainminder/pull/6#issuecomment-182447516

.

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub < https://github.com/google/macops-keychainminder/pull/6#issuecomment-182457644

.

— Reply to this email directly or view it on GitHub https://github.com/google/macops-keychainminder/pull/6#issuecomment-182464946 .

[howiekaye]

OK, Switched that back to YES.