-
Notifications
You must be signed in to change notification settings - Fork 464
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
Repeated keychain prompts in v1.10.2 & 1.11.0 #17064
Comments
@leighghunt thanks for reporting! It shouldn't be related to this being a preview build so we'll investigate to ensure this doesn't also impact the upcoming 1.11.0 build. |
I don't use the preview build but still receive the promt and when I check in the changelog there is no mention about it. Can we include the reason why this plugin need to update the keychain and include it in the changelog? |
I'm also seeing this every time I start VSCode. I disabled the extension and the popup is no longer shown, so I'm sure it coming from this extension. It looks like it auto upgraded to v1.11.0 this morning, so I installed v.1.10.1 and that seams to be a workaround for now. |
Seeing this same. I was mostly checking to make sure this wasn't something bad, but yeah having the prompt each time vscode starts is... non-optimal. LMK if I can send info to help debug this. |
Same issue for me with v1.11.0. Though for me it checks for keychain entry "a". |
Got this message every time I tried to run a query this morning (v1.11.0 on Mac OS 10.15.7), making the new version essentially unusable if you deny the permissions. Downgraded to v.1.10.1 for fix. |
@cssuh can you take a look if anything's changed here in 1.11.0? I think when we push an update users will need to reenter their keychain password to access saved passwords using the new binary, but this should be a one-time (or at least one-time per password). Not something that keep reoccurring. |
A few follow-up questions/responses regarding previous comments... @chigia001 this extension saves credentials in the keystore for saved connections. You should only get this prompt when connecting to a server, so if it's happening outside that scenario then that sounds like an unintended design change. When the extension is updated the binary that reads these keys is updated and needs to be granted permissions to the keychain again. @sharumpe @boxofyellow it's strange that this is happening on startup if you aren't connecting to a server. Chris will investigate what's going on there. Are you granting SQL Tools Service access and then it continues to prompt subsequently, or are you denying STS permissions? @benzdouglas you would need to grant STS permissions to the keychain to read saved credential information and establish a connection. Though this should be a one-time thing after upgrade. Downgrading may workaround this issue since the previous version of STS may already have permissions to the keychain. Please let me know if we're missing any details with this issue. Thanks! |
I have been clicking "Deny", but I just restarted to have a go at it again. I was prompted a total of two times for the same thing (wants access to "a" in the "login" keychain). I entered my creds and clicked "Allow". Both ways (allow and deny) seem to end up with me being able to use the connections I've got set up on the SQL Server tool pane. |
@sharumpe thanks for the additional context. It's interesting the connect still works when you click Deny. We'll continue to investigate. |
Some additional info, which may (or may not) help, I think I uninstalled the preview release back when I experienced the issue - and now appear to be using the normal v1.10.2 version (not via preview) and I'm not experiencing the issue. I then updated to version v1.11.0 as suggested by extension, and again no problems with the keychain prompts. Out of curiosity, I uninstalled the extension, and attempted to redownload the preview version again from https://github.com/microsoft/vscode-mssql/releases/tag/v1.10.2, to verify the issue still occurred, but it seems to be automatically updating to v1.11.0 - I can no longer reproduce the issue. Please let me know if you'd like me to try any tests. |
I've been able to reproduce this now in my environment. I've typically selected "Always Allow" which makes the prompt go away, which is why I wasn't seeing this. There is some change in 1.11.0 where we seem to be unnecessarily trying to read credentials on startup. We should access the keychain when trying to connect and it's necessary to retrieve the saved encrypted credentials. We're investigating what's changed and will likely push a 1.11.1 update with the fix. Obviously selecting "Always Allow" should suppress this prompt. Though I certainly understand if users prefer to be prompted each time their keychain is accessed. For what it's worth, you can review the code we use to access the keychain here and in related files if there is any concern regarding that https://github.com/microsoft/sqltoolsservice/blob/main/src/Microsoft.SqlTools.Credentials/Credentials/OSX/OSXCredentialStore.cs. |
I had the opportunity to do a little more thorough testing this morning, and I need to correct my earlier statement: |
@sharumpe thanks for the clarification. It was a bit of a mystery (concern) how the connection would work after denying the prompt. We're investigating why we're trying to read saved credentials when we don't need to (basically any time other than when establishing a connection). |
Just started getting the same issue today as well after rebooting. |
The 1.11.1 release we published today should no longer have a keystore prompt on vscode startup. Please let us know if this is not the case in your environment. Thanks! |
I'm experiencing this on v1.13.0. I've always clicked "Always allow" and the prompt goes away usually after the SECOND time of going through the process of entering my PW and clicking "Always Allow". Here's what's different though:
Is there a way to set something so that "always allow" actually means ALWAYS? Even after an update is installed? |
Steps to Reproduce:
Note - if this is simply because it's a preview release and is perhaps unsigned, or something like that kicking off MacOSX being fussy, happy to close ticket.
Thanks for the work on getting it working on M1.
The text was updated successfully, but these errors were encountered: