[COMPASS-3235] Eliminate multiple keychain password requests Created: 06/Nov/18  Updated: 29/Oct/23  Resolved: 10/Sep/19

Status: Closed
Project: Compass
Component/s: Connectivity, Favorites, Security
Affects Version/s: 1.19.0
Fix Version/s: 1.20.0

Type: Task Priority: Critical - P2
Reporter: Lucas Hrabovsky (Inactive) Assignee: Durran Jordan
Resolution: Fixed Votes: 1
Labels: security
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File Screen Shot 2019-08-13 at 10.09.43 AM.png     File keychain-password-requests.mov    
Issue Links:
Documented
Related
is related to COMPASS-3469 Compass Not Pulling Passwords from Ke... Closed
is related to COMPASS-3091 Investigate Password Storage Regressi... Closed
is related to COMPASS-3147 Clicking allow, deny, or always allow... Closed
is related to COMPASS-3710 Connections/Favorites are not persist... Closed
is related to COMPASS-1767 Keychain approval appears many times ... Closed
is related to COMPASS-3215 Repeated request for keychain password Closed
Epic Link: COMPASS-3417
Story Points: 3
Documentation Changes: Needed
Documentation Changes Summary:

It's worth including in the release notes for Compass 1.20 that on OSX, after the upgrade, Compass will ask for the keychain password a few times (depending on the number of favorites). In this release, we changed the process that loads the favorites and our expectation is that this is the last time the user will have to enter their keychain password after an update.

Sprint: Iteration Bison, Iteration Manatee

 Description   

The multiple keychain password prompt has been a long-running unexplained behavior we haven't been able to get to the bottom of. A roll-up of bug reports:

My hunch after re-reading these tickets is this happens after an auto-update or a manual upgrade. What's most likely is that we simply need to call keytar methods from the main process via ipc rather than from the renderer as we do today. See this example on stackoverflow.

From this blog post:

One other important note: I recommend you only call node-keytar from the main process. If you set a password from the main process and then attempt to get it from a renderer process, it’ll prompt a permissions dialog for the user (this is macOS only, Windows doesn’t seem to mind either way). Additionally, I think it’s cleaner and clearer to the user if the access control list has your app name and it’s icon, instead of MyApp Helper and the generic app icon which is what you get when a renderer sets it.

More notes from previous tickets rolled up below.

SecKeychainFindGenericPassword, which is the method keytar uses to read a stored connection password. In the discussion:

This function automatically calls the function SecKeychainUnlock to display the Unlock Keychain dialog box if the keychain is currently locked.

A few ideas on what might need to happen:

  • Maybe something in the keytar bindings is too specific?
    Maybe when we run app-migrations today, macOS needs to re-validate or something?
  • Maybe a bulk-read call to fetch all passwords with FindPassword would guarantee this unlock dialog is shown once and only once in all cases (a single, implicit SecKeychainUnlock call), but there are some potential security implications to consider.


 Comments   
Comment by Eliezer Steinbock [ 13/Aug/19 ]

This still seems to be an issue. Since downloading 1.19.0 I cannot use Compass anymore. It keeps asking for keychain password.

Comment by Durran Jordan [ 09/Apr/19 ]

Have not seen this on master in a few weeks.

Comment by Githook User [ 29/Mar/19 ]

Author:

{'email': 'durran@gmail.com', 'name': 'Durran Jordan', 'username': 'durran'}

Message: COMPASS-3546: Fix Connections Requiring Password:

The connection model version was not updated in the data-service to the
models that both Compass and the data-service were using were actually
different. Compass was using an extended model and the data-service was
using the old undecorated model. This meant where the actuall connection
was happening in the data-service the model was not properly retrieving
the username and password from the secure storage.

This fixes that issue, and also removes the cyclic dependency between
the extended connection model and the data-service. The #test method was
not used in Compass thus the data-service connection testing
functionality in the model itself was no longer needed. This must only
happen in the data service.

This also brings the keytar versions in the data-service and Compass in
line. Could potentially fix COMPASS-3235. I don't see this behaviour
anymore but will give it some time. This also fixes the tests in the
data-service since the electron require in the extended models now
protect against failure to load electron in pure node envs.
Branch: master
https://github.com/10gen/compass/commit/79c7a05b35a123c7d17aec51e9b5671209939ded

Comment by Githook User [ 29/Mar/19 ]

Author:

{'email': 'durran@gmail.com', 'name': 'Durran Jordan', 'username': 'durran'}

Message: COMPASS-3546: Fix Connections Requiring Password:

The connection model version was not updated in the data-service to the
models that both Compass and the data-service were using were actually
different. Compass was using an extended model and the data-service was
using the old undecorated model. This meant where the actuall connection
was happening in the data-service the model was not properly retrieving
the username and password from the secure storage.

This fixes that issue, and also removes the cyclic dependency between
the extended connection model and the data-service. The #test method was
not used in Compass thus the data-service connection testing
functionality in the model itself was no longer needed. This must only
happen in the data service.

This also brings the keytar versions in the data-service and Compass in
line. Could potentially fix COMPASS-3235. I don't see this behaviour
anymore but will give it some time. This also fixes the tests in the
data-service since the electron require in the extended models now
protect against failure to load electron in pure node envs.
Branch: fix-connect
https://github.com/10gen/compass/commit/913f64c8c13d5482bb1a1c45a918f5a9dcf4d633

Generated at Wed Feb 07 22:32:35 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.