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

Please move username and password data into separate file #3

Closed
ALIENQuake opened this issue Nov 10, 2019 · 16 comments
Closed

Please move username and password data into separate file #3

ALIENQuake opened this issue Nov 10, 2019 · 16 comments

Comments

@ALIENQuake
Copy link

@ALIENQuake ALIENQuake commented Nov 10, 2019

So users can exclude it when syncing VSCode user configuration with Github.

@znck

This comment has been minimized.

Copy link
Owner

@znck znck commented Nov 14, 2019

Is there a known way to do this?

@ALIENQuake

This comment has been minimized.

Copy link
Author

@ALIENQuake ALIENQuake commented Nov 14, 2019

@znck
You can simply add grammarly.privatesettings configuration item with default to FolderWithVSCodeUserSettings\settings-grammarly.json then read and write username and password there instead of VSCode main settings.json.

@znck znck added the enhancement label Nov 21, 2019
@znck znck added this to To do in Roadmap Nov 21, 2019
@znck znck moved this from To do to In progress in Roadmap Nov 21, 2019
@znck znck added this to High priority in Bug Triaging Nov 21, 2019
@znck znck closed this in a41f077 Jan 10, 2020
Roadmap automation moved this from In progress to Done Jan 10, 2020
Bug Triaging automation moved this from High priority to Closed Jan 10, 2020
@znck znck moved this from Done to In progress in Roadmap Jan 10, 2020
@znck znck reopened this Jan 10, 2020
Bug Triaging automation moved this from Closed to Needs triage Jan 10, 2020
@mwz

This comment has been minimized.

Copy link

@mwz mwz commented Feb 2, 2020

@ALIENQuake, I can't get this to work.
Are you saying that in my vscode settings settings.json file I should add

"grammarly.privatesettings": "~/Library/Application Support/Code/User/settings-grammarly.json"

and extract all the grammarly settings there, like so?

{
  "grammarly.dialect": "british",
  "grammarly.domain": "technical",
  "grammarly.password": "redacted",
  "grammarly.username": "redacted",
  "grammarly.userWords": []
}

The settings key grammarly.privatesettings doesn't seem to be recognised as a valid key and the extension doesn't seem to be connecting to my account, i.e. I can't add any custom words to my user dictionary.

@ALIENQuake

This comment has been minimized.

Copy link
Author

@ALIENQuake ALIENQuake commented Feb 2, 2020

@mwz The issue is not closed so I assume that it isn't yet implemented unless I miss something.

@znck

This comment has been minimized.

Copy link
Owner

@znck znck commented Feb 3, 2020

It's not implemented yet.

@andrew-codes

This comment has been minimized.

Copy link
Contributor

@andrew-codes andrew-codes commented Mar 10, 2020

Would saving the settings using the global storage path be the correct approach? I think it provides a directory for each extension; with both read and write access.

@ALIENQuake

This comment has been minimized.

Copy link
Author

@ALIENQuake ALIENQuake commented Mar 10, 2020

@andrew-codes Sure, everything is better than user config.

@andrew-codes

This comment has been minimized.

Copy link
Contributor

@andrew-codes andrew-codes commented Mar 10, 2020

I'm going to take a stab at this later today and submit a PR. I, too, sync my settings to Gists. Storing the credentials in settings is a blocker for me also. From a design perspective and desired patterns for this code base, are there any considerations I should take @znck ?

@znck

This comment has been minimized.

Copy link
Owner

@znck znck commented Mar 11, 2020

I was planning to store cookie in some place and prompt user for credentials. By storing cookie, it would reuse session that mean same container and document namespace hence documents preserve manuals fixes even after vscode restart (just a hypothesis, need to test).

As per conventions, just use prettier.

@andrew-codes

This comment has been minimized.

Copy link
Contributor

@andrew-codes andrew-codes commented Mar 11, 2020

@znck Will storing it in a cookie enable the same credentials across different instances and workspaces of VS Code, too? It does appear to be beneficial to use the same container/namespace if it preserves manual fixes to documents across restarts. Are you thinking of storing the credentials in the cookie or the output of using the credentials when authenticating?

I'll try the cookie approach first and test the hypothesis and resort to a settings file in the global storage path as a last-ditch option. I wonder what best practices are out there for storing sensitive information for VSCode extensions. It may be a good idea to look into that as well.

@znck

This comment has been minimized.

Copy link
Owner

@znck znck commented Mar 11, 2020

In current auth process, it collects cookie header on successful authentication and use the cookie in subsequent requests.

We still have to use a global store to store the cookie.

@znck

This comment has been minimized.

Copy link
Owner

@znck znck commented Mar 11, 2020

Check grammarly-auth.ts for auth related code.

@andrew-codes

This comment has been minimized.

Copy link
Contributor

@andrew-codes andrew-codes commented Mar 11, 2020

Ah ok. I misunderstood. I thought you wanted to store the username/password in a cookie within VS Code, so I wasn't following that train of thought. However, is this a correct summary? Store the username/password AND any relevant cookie data to a separate place/file instead of storing (only) username/password in VS Code's settings.

If the above is correct, then I think I understand your intentions and can open a PR for it (hopefully later today or tomorrow).

@ALIENQuake

This comment has been minimized.

Copy link
Author

@ALIENQuake ALIENQuake commented Mar 11, 2020

@andrew-codes Yes, that's the correct intention, at the very least from the perspective of my request.

@znck

This comment has been minimized.

Copy link
Owner

@znck znck commented Mar 12, 2020

👍 @andrew-codes Please go ahead with the PR

@andrew-codes

This comment has been minimized.

Copy link
Contributor

@andrew-codes andrew-codes commented Mar 14, 2020

I did a little research and found a recommended way to store secrets like username, passwords, tokens, etc. It is to use keytar that already ships with VS Code. I adapted some code from the extension vscode-pull-request-github (src).

However, I am beginning to run into a strange issue. I am now experiencing this issue when using your master branch (including blowing away and re-installing node_modules). I'll mention the problem in the PR I open, and maybe we can determine if this is my environment only, the code, or some mixture of the two.

@znck znck closed this in #50 Mar 18, 2020
Roadmap automation moved this from In progress to Done Mar 18, 2020
Bug Triaging automation moved this from Needs triage to Closed Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Roadmap
  
Done
Bug Triaging
  
Closed
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.