Document when gh auth login writes oauth token to plain text#7781
Merged
Document when gh auth login writes oauth token to plain text#7781
Conversation
Adds documentation to gh auth login about plain text fallback. Prints a warning when it is stored in plain text. Fixes cli#7757
josebalius
reviewed
Aug 2, 2023
Contributor
josebalius
left a comment
There was a problem hiding this comment.
left some initial comments
internal/config/config.go
Outdated
Comment on lines
+231
to
+235
| if !secureStorage || setErr != nil { | ||
| c.cfg.Set([]string{hosts, hostname, oauthToken}, token) | ||
| insecureStorageUsed = true | ||
| } else { | ||
| insecureStorageUsed = false |
Contributor
There was a problem hiding this comment.
Suggested change
| if !secureStorage || setErr != nil { | |
| c.cfg.Set([]string{hosts, hostname, oauthToken}, token) | |
| insecureStorageUsed = true | |
| } else { | |
| insecureStorageUsed = false | |
| insecureStorageUsed = false | |
| if !secureStorage || setErr != nil { | |
| c.cfg.Set([]string{hosts, hostname, oauthToken}, token) | |
| insecureStorageUsed = true | |
| } |
internal/config/config.go
Outdated
| // If the encrypt option is specified it will first try to store the auth token | ||
| // in encrypted storage and will fall back to the plain text config file. | ||
| func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secureStorage bool) error { | ||
| func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secureStorage bool) (configWriteErr error, insecureStorageUsed bool) { |
Contributor
There was a problem hiding this comment.
I find this return signature a bit non-idiomatic. What do you think about flipping the return args so the bool is first and error second?
Also, do we need to use named return args?
Contributor
Author
There was a problem hiding this comment.
Sure that makes sense. I've updated the pull request.
samcoe
suggested changes
Aug 2, 2023
Contributor
samcoe
left a comment
There was a problem hiding this comment.
@kbailey4444 Thanks for getting started on this work, I added a couple comments mostly regarding UX and verbiage. Let me know if any of my comments are unclear.
Co-authored-by: Sam Coe <samcoe@users.noreply.github.com>
samcoe
approved these changes
Sep 18, 2023
Contributor
samcoe
left a comment
There was a problem hiding this comment.
@kbailey4444 Sorry for the delayed review, this looks good to me. Thanks for the contribution and making the requested changes.
renovate bot
referenced
this pull request
in scottames/dots
Sep 22, 2023
[](https://renovatebot.com) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [aquaproj/aqua-registry](https://togithub.com/aquaproj/aqua-registry) | minor | `v4.48.2` -> `v4.52.0` | | [cli/cli](https://togithub.com/cli/cli) | minor | `v2.34.0` -> `v2.35.0` | | [fluxcd/flux2](https://togithub.com/fluxcd/flux2) | patch | `v2.1.0` -> `v2.1.1` | | [twpayne/chezmoi](https://togithub.com/twpayne/chezmoi) | minor | `v2.39.1` -> `v2.40.0` | | [weaveworks/eksctl](https://togithub.com/weaveworks/eksctl) | minor | `v0.157.0` -> `v0.158.0` | --- ### Release Notes <details> <summary>aquaproj/aqua-registry (aquaproj/aqua-registry)</summary> ### [`v4.52.0`](https://togithub.com/aquaproj/aqua-registry/releases/tag/v4.52.0) [Compare Source](https://togithub.com/aquaproj/aqua-registry/compare/v4.51.0...v4.52.0) [Issues](https://togithub.com/aquaproj/aqua-registry/issues?q=is%3Aissue+milestone%3Av4.52.0) | [Pull Requests](https://togithub.com/aquaproj/aqua-registry/pulls?q=is%3Apr+milestone%3Av4.52.0) | aquaproj/aqua-registry@v4.51.0...v4.52.0 #### 🎉 New Packages [#​15604](https://togithub.com/aquaproj/aqua-registry/issues/15604) [natecraddock/zf](https://togithub.com/natecraddock/zf): a commandline fuzzy finder designed for filtering filepaths [@​4513ECHO](https://togithub.com/4513ECHO) [#​15600](https://togithub.com/aquaproj/aqua-registry/issues/15600) [winebarrel/cronplan/{cronmatch,cronplan,cronviz}](https://togithub.com/winebarrel/cronplan): Cron expression parser for Amazon EventBridge [@​ponkio-o](https://togithub.com/ponkio-o) #### Fixes [#​15605](https://togithub.com/aquaproj/aqua-registry/issues/15605) cloudflare/cfssl: Rename the package cloudflare/cfssl to cloudflare/cfssl/cfssl [@​ponkio-o](https://togithub.com/ponkio-o) [#​15602](https://togithub.com/aquaproj/aqua-registry/issues/15602) protocolbuffers/protobuf/protoc: Support arm64 [@​4513ECHO](https://togithub.com/4513ECHO) [#​15606](https://togithub.com/aquaproj/aqua-registry/issues/15606) protocolbuffers/protobuf/protoc: Exclude windows/arm64 [@​4513ECHO](https://togithub.com/4513ECHO) ### [`v4.51.0`](https://togithub.com/aquaproj/aqua-registry/releases/tag/v4.51.0) [Compare Source](https://togithub.com/aquaproj/aqua-registry/compare/v4.50.0...v4.51.0) [Issues](https://togithub.com/aquaproj/aqua-registry/issues?q=is%3Aissue+milestone%3Av4.51.0) | [Pull Requests](https://togithub.com/aquaproj/aqua-registry/pulls?q=is%3Apr+milestone%3Av4.51.0) | aquaproj/aqua-registry@v4.50.0...v4.51.0 #### 🎉 New Packages [#​15578](https://togithub.com/aquaproj/aqua-registry/issues/15578) [greymd/ojichat](https://togithub.com/greymd/ojichat) [@​ryoppippi](https://togithub.com/ryoppippi) [#​15583](https://togithub.com/aquaproj/aqua-registry/issues/15583) [koron/iferr](https://togithub.com/koron/iferr): Generate "if err != nil {" block [@​ryoppippi](https://togithub.com/ryoppippi) [#​15577](https://togithub.com/aquaproj/aqua-registry/issues/15577) [mattn/zig-update](https://togithub.com/mattn/zig-update) [@​ryoppippi](https://togithub.com/ryoppippi) [#​15582](https://togithub.com/aquaproj/aqua-registry/issues/15582) [mike-engel/jwt-cli](https://togithub.com/mike-engel/jwt-cli): A super fast CLI tool to decode and encode JWTs built in Rust [@​ryoppippi](https://togithub.com/ryoppippi) [#​15592](https://togithub.com/aquaproj/aqua-registry/issues/15592) [simonwhitaker/gibo](https://togithub.com/simonwhitaker/gibo): Easy access to gitignore boilerplates [@​ryoppippi](https://togithub.com/ryoppippi) [#​15580](https://togithub.com/aquaproj/aqua-registry/issues/15580) [tmc/json-to-struct](https://togithub.com/tmc/json-to-struct): A simple command-line tool for generating to struct definitions from JSON [@​ryoppippi](https://togithub.com/ryoppippi) [#​15579](https://togithub.com/aquaproj/aqua-registry/issues/15579) [tomohiro/gyazo-cli](https://togithub.com/tomohiro/gyazo-cli): Gyazo command-line uploader [@​ryoppippi](https://togithub.com/ryoppippi) ### [`v4.50.0`](https://togithub.com/aquaproj/aqua-registry/releases/tag/v4.50.0) [Compare Source](https://togithub.com/aquaproj/aqua-registry/compare/v4.49.0...v4.50.0) [Issues](https://togithub.com/aquaproj/aqua-registry/issues?q=is%3Aissue+milestone%3Av4.50.0) | [Pull Requests](https://togithub.com/aquaproj/aqua-registry/pulls?q=is%3Apr+milestone%3Av4.50.0) | aquaproj/aqua-registry@v4.49.0...v4.50.0 #### 🎉 New Packages [#​15536](https://togithub.com/aquaproj/aqua-registry/issues/15536) [notaryproject/notation](https://togithub.com/notaryproject/notation): A CLI tool to sign and verify artifacts [#​15543](https://togithub.com/aquaproj/aqua-registry/issues/15543) [uzimaru0000/oglens](https://togithub.com/uzimaru0000/oglens): Tools for viewing OGP [@​ryoppippi](https://togithub.com/ryoppippi) #### Fixes [#​15534](https://togithub.com/aquaproj/aqua-registry/issues/15534) kdash-rs/kdash: Follow up changes of kdash v0.4.3 [#​15553](https://togithub.com/aquaproj/aqua-registry/issues/15553) norwoodj/helm-docs: Follow up changes of helm-docs v1.11.2 Assets were renamed. - [https://github.com/norwoodj/helm-docs/pull/181](https://togithub.com/norwoodj/helm-docs/pull/181) - norwoodj/helm-docs@297d05b ### [`v4.49.0`](https://togithub.com/aquaproj/aqua-registry/releases/tag/v4.49.0) [Compare Source](https://togithub.com/aquaproj/aqua-registry/compare/v4.48.2...v4.49.0) [Issues](https://togithub.com/aquaproj/aqua-registry/issues?q=is%3Aissue+milestone%3Av4.49.0) | [Pull Requests](https://togithub.com/aquaproj/aqua-registry/pulls?q=is%3Apr+milestone%3Av4.49.0) | aquaproj/aqua-registry@v4.48.2...v4.49.0 #### 🎉 New Packages [#​15503](https://togithub.com/aquaproj/aqua-registry/issues/15503) [rustwasm/wasm-pack](https://togithub.com/rustwasm/wasm-pack): your favorite rust -> wasm workflow tool [@​4513ECHO](https://togithub.com/4513ECHO) #### Fixes [#​15501](https://togithub.com/aquaproj/aqua-registry/issues/15501) noahgorstein/jqp: Follow up changes of jqp v0.5.0 Assets were renamed. - [https://github.com/noahgorstein/jqp/pull/43](https://togithub.com/noahgorstein/jqp/pull/43) </details> <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.35.0`](https://togithub.com/cli/cli/releases/tag/v2.35.0): GitHub CLI 2.35.0 [Compare Source](https://togithub.com/cli/cli/compare/v2.34.0...v2.35.0) #### What's New - `gh alias delete` now supports the `--all` flag by [@​JunNishimura](https://togithub.com/JunNishimura) in [https://github.com/cli/cli/pull/7900](https://togithub.com/cli/cli/pull/7900) - `gh release delete` now supports the `--cleanup-tag` flag by [@​kemingy](https://togithub.com/kemingy) in [https://github.com/cli/cli/pull/7884](https://togithub.com/cli/cli/pull/7884) - `gh release create` now supports the `--notes-from-tag` flag by [@​kbailey4444](https://togithub.com/kbailey4444) in [https://github.com/cli/cli/pull/7861](https://togithub.com/cli/cli/pull/7861) #### What's Changed - Clarified `gh repo list --fork` and `--source` behavior for orgs [@​ncalteen](https://togithub.com/ncalteen) in [https://github.com/cli/cli/pull/7964](https://togithub.com/cli/cli/pull/7964) - `gh cs create` now shows the full permissions URL by [@​joshmgross](https://togithub.com/joshmgross) in [https://github.com/cli/cli/pull/7983](https://togithub.com/cli/cli/pull/7983) - Documented when `gh auth login` falls back to using insecure storage by [@​kbailey4444](https://togithub.com/kbailey4444) in [https://github.com/cli/cli/pull/7781](https://togithub.com/cli/cli/pull/7781) - Bumped goreleaser/goreleaser-action from 4 to 5 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/cli/cli/pull/7981](https://togithub.com/cli/cli/pull/7981) #### New Contributors - [@​kemingy](https://togithub.com/kemingy) made their first contribution in [https://github.com/cli/cli/pull/7884](https://togithub.com/cli/cli/pull/7884) - [@​ncalteen](https://togithub.com/ncalteen) made their first contribution in [https://github.com/cli/cli/pull/7964](https://togithub.com/cli/cli/pull/7964) - [@​kbailey4444](https://togithub.com/kbailey4444) made their first contribution in [https://github.com/cli/cli/pull/7861](https://togithub.com/cli/cli/pull/7861) **Full Changelog**: cli/cli@v2.34.0...v2.35.0 </details> <details> <summary>fluxcd/flux2 (fluxcd/flux2)</summary> ### [`v2.1.1`](https://togithub.com/fluxcd/flux2/releases/tag/v2.1.1) [Compare Source](https://togithub.com/fluxcd/flux2/compare/v2.1.0...v2.1.1) #### Highlights Flux `v2.1.1` is a patch release which comes with various fixes. Users are encouraged to upgrade for the best experience. ##### Fixes - Use auto lookup strategy for Buckets to widen support for S3-compatible object storage services (`source-controller`). - Fix Secret type check for HelmRepositories TLS certs referred in `.spec.secretRef` (`source-controller`). - Fix the branch name reporting when the push branch is the same as the checkout branch (`image-automation-controller`). - Restore Helm logs inclusion in failure events (`helm-controller`). - Fix the impersonation of the default service account when diffing HelmReleases (`helm-controller`). - Check source for `nil` artifact before loading Helm charts (`helm-controller`). - Update the description of Kubernetes specific flag to distinguish them from Flux bootstrap flags (`flux` CLI). #### Components changelog - source-controller [v1.1.1](https://togithub.com/fluxcd/source-controller/blob/v1.1.1/CHANGELOG.md) - helm-controller [v0.36.1](https://togithub.com/fluxcd/helm-controller/blob/v0.36.1/CHANGELOG.md) - image-automation-controller [v0.36.1](https://togithub.com/fluxcd/image-automation-controller/blob/v0.36.1/CHANGELOG.md) #### CLI Changelog - PR [#​4255](https://togithub.com/fluxcd/flux2/issues/4255) - [@​hiddeco](https://togithub.com/hiddeco) - tests/azure: update controller dependencies - PR [#​4251](https://togithub.com/fluxcd/flux2/issues/4251) - [@​fluxcdbot](https://togithub.com/fluxcdbot) - Update toolkit components - PR [#​4246](https://togithub.com/fluxcd/flux2/issues/4246) - [@​dependabot](https://togithub.com/dependabot)\[bot] - build(deps): bump the ci group with 4 updates - PR [#​4238](https://togithub.com/fluxcd/flux2/issues/4238) - [@​makkes](https://togithub.com/makkes) - Upgrade github.com/fluxcd/pkg/{git,git/gogit} - PR [#​4233](https://togithub.com/fluxcd/flux2/issues/4233) - [@​sonbui00](https://togithub.com/sonbui00) - chore: remove support armv6h for aur package - PR [#​4228](https://togithub.com/fluxcd/flux2/issues/4228) - [@​sonbui00](https://togithub.com/sonbui00) - Improve AUR package templates - PR [#​4227](https://togithub.com/fluxcd/flux2/issues/4227) - [@​dependabot](https://togithub.com/dependabot)\[bot] - build(deps): bump the ci group with 3 updates - PR [#​4226](https://togithub.com/fluxcd/flux2/issues/4226) - [@​somtochiama](https://togithub.com/somtochiama) - Update description of kubeconfig specific flag - PR [#​4222](https://togithub.com/fluxcd/flux2/issues/4222) - [@​dependabot](https://togithub.com/dependabot)\[bot] - build(deps): bump github.com/cyphar/filepath-securejoin from 0.2.3 to 0.2.4 in /tests/integration - PR [#​4221](https://togithub.com/fluxcd/flux2/issues/4221) - [@​dependabot](https://togithub.com/dependabot)\[bot] - build(deps): bump github.com/cyphar/filepath-securejoin from 0.2.3 to 0.2.4 in /tests/azure - PR [#​4215](https://togithub.com/fluxcd/flux2/issues/4215) - [@​dependabot](https://togithub.com/dependabot)\[bot] - build(deps): bump the ci group with 4 updates - PR [#​4213](https://togithub.com/fluxcd/flux2/issues/4213) - [@​dependabot](https://togithub.com/dependabot)\[bot] - build(deps): bump github.com/docker/distribution from 2.8.1+incompatible to 2.8.2+incompatible in /tests/integration - PR [#​4212](https://togithub.com/fluxcd/flux2/issues/4212) - [@​dependabot](https://togithub.com/dependabot)\[bot] - build(deps): bump github.com/docker/docker from 23.0.1+incompatible to 23.0.3+incompatible in /tests/integration - PR [#​4198](https://togithub.com/fluxcd/flux2/issues/4198) - [@​makkes](https://togithub.com/makkes) - Add 2.1.x backport label - PR [#​4197](https://togithub.com/fluxcd/flux2/issues/4197) - [@​stefanprodan](https://togithub.com/stefanprodan) - Fix links to fluxcd.io - PR [#​4195](https://togithub.com/fluxcd/flux2/issues/4195) - [@​dependabot](https://togithub.com/dependabot)\[bot] - build(deps): bump the ci group with 2 updates </details> <details> <summary>twpayne/chezmoi (twpayne/chezmoi)</summary> ### [`v2.40.0`](https://togithub.com/twpayne/chezmoi/releases/tag/v2.40.0) [Compare Source](https://togithub.com/twpayne/chezmoi/compare/v2.39.1...v2.40.0) #### Changelog ##### Features - [`2858a0c`](https://togithub.com/twpayne/chezmoi/commit/2858a0c8) feat: Implement the path-style flag for status - [`5918296`](https://togithub.com/twpayne/chezmoi/commit/59182964) feat: Add plugin support - [`63cda81`](https://togithub.com/twpayne/chezmoi/commit/63cda81c) feat: Allow overlapping, non-conflicting externals - [`f15b158`](https://togithub.com/twpayne/chezmoi/commit/f15b158d) feat: Add decryption of non-armored files to age command ##### Fixes - [`cdd4f16`](https://togithub.com/twpayne/chezmoi/commit/cdd4f166) fix: Use diff pager for all diff output if configured - [`3667788`](https://togithub.com/twpayne/chezmoi/commit/36677884) fix: provide a consistent error for cd to file ##### Documentation updates - [`284baf6`](https://togithub.com/twpayne/chezmoi/commit/284baf6a) docs: Add links to articles </details> <details> <summary>weaveworks/eksctl (weaveworks/eksctl)</summary> ### [`v0.158.0`](https://togithub.com/eksctl-io/eksctl/releases/tag/v0.158.0): eksctl 0.158.0 (permalink) [Compare Source](https://togithub.com/weaveworks/eksctl/compare/0.157.0...0.158.0-rc.0) ### Release v0.158.0 #### 🐛 Bug Fixes - Call `kubectl version` command with `--output=json` flag ([#​7032](https://togithub.com/weaveworks/eksctl/issues/7032)) #### 🧰 Maintenance - Bump `github.com/cloudflare/cfssl` to fix indirect deps DOS vulnerability ([#​7067](https://togithub.com/weaveworks/eksctl/issues/7067)) - Upgrade goproxy to fix DoS vulnerability ([#​7068](https://togithub.com/weaveworks/eksctl/issues/7068)) #### 📝 Documentation - Corrected Chocolatey Install Command ([#​7063](https://togithub.com/weaveworks/eksctl/issues/7063)) #### Acknowledgments Weaveworks would like to sincerely thank: [@​Nyami](https://togithub.com/Nyami) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 4pm on thursday" in timezone America/Los_Angeles, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://togithub.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/scottames/dots). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi45Ny4xIiwidXBkYXRlZEluVmVyIjoiMzYuOTcuMSIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
dmgardiner25
pushed a commit
to dmgardiner25/cli
that referenced
this pull request
Sep 25, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds documentation to gh auth login about plain text fallback.
Prints a warning when it is stored in plain text.
Fixes #7757