zulip-terminal
zulip-terminal copied to clipboard
pass command instead of password in zuliprc
What does this PR do, and why?
This PR updates the default config generation logic in zulip-term to support a more secure way of managing Zulip API keys. Instead of writing the API key directly into the zuliprc file, we now generate: • a zuliprc file with a passcmd field • a zulip_key file containing just the API key • zulip_key: A temporary file that contains only the API key. • zuliprc: [api]: contains Zulip login configuration (email, server URL, and now passcmd)
Here’s how the new auto-generated zuliprc file looks:
[api]
[email protected]
# Fill the passcmd field with a command that outputs the API key.
# The API key is temporarily stored in the 'zulip_key' file.
# After storing the key in a password manager, please delete the 'zulip_key' file.
passcmd=cat zulip_key
site=https://chat.zulip.org
If you’re downloading a zuliprc file from your Zulip account (which includes the key field), you’re encouraged to manually replace the key line with a passcmd, like this:
passcmd=cat zulip_key
You can (and should) later replace cat zulip_key with a command tied to your preferred secret storage tool
Outstanding aspect(s)
- [ ]
External discussion & connections
- [x] Discussed in #zulip-terminal in
topic - [x] Fully fixes #1502
- [ ] Partially fixes issue #
- [ ] Builds upon previous unmerged work in PR #
- [ ] Is a follow-up to work in PR #
- [ ] Requires merge of PR #
- [ ] Merge will enable work on #
How did you test this?
- [x] Manually - Behavioral changes
- [ ] Manually - Visual changes
- [ ] Adapting existing automated tests
- [x] Adding automated tests for new behavior (or missing tests)
- [ ] Existing automated tests should already cover this (only a refactor of tested code)
Self-review checklist for each commit
- [x] It is a minimal coherent idea
- [ ] It has a commit summary following the documented style (title & body)
- [ ] It has a commit summary describing the motivation and reasoning for the change
- [x] It individually passes linting and tests
- [x] It contains test additions for any new behavior
- [ ] It flows clearly from a previous branch commit, and/or prepares for the next commit
Hello @Gopinath-Mahendiran, it seems like you have referenced #1502 in your pull request description, but you have not referenced them in your commit message description(s). Referencing an issue in a commit message automatically closes the corresponding issue when the commit is merged, which makes the issue tracker easier to manage.
Please run git commit --amend in your command line client to amend your commit message description with Fixes #1502..
An example of a correctly-formatted commit:
commit fabd5e450374c8dde65ec35f02140383940fe146
Author: zulipbot
Date: Sat Mar 18 13:42:40 2017 -0700
pull requests: Check PR commits reference when issue is referenced.
Fixes #51.
To learn how to write a great commit message, please refer to our guide.
@neiljp,@dkwo review the pr and kindly give the feedback
I will work towards the changes
@neiljp, updated the code as discussed. Let me know if it looks good.
@Gopinath-Mahendiran I reviewed zulip/python-zulip-api#866, and of course this PR will depend on those changes to at least some extent.
Generally this PR would be easier to follow if you can rework the commits from what in the README are termed 'developmental-style commits', into commits that focus on the changes more cleanly. Specifically there are still changes that seem unrelated, as well as changes within the PR that fix earlier errors within the PR? The latter is fine if it lays out how you change things step by step, but not so much if something was missing in an earlier commit - it's clearer to correct the original commit.
@neiljp now the commits are tied up and have a look at zulip/python-zulip-api#866
I believe it is no longer necessary to specify whether the user will utilize the key or passcmd. Based on the recent changes in the python-zulip-api, either method is now accepted seamlessly. Users can either rely on the default zuliprc or use passcmd; as long as one of them is present, the application will function correctly without any disruption.
Since the README.md requires some modifications, I will update it based on your feedback