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

Make auth refresh more convenient with secure storage #7098

Merged
merged 1 commit into from
Mar 7, 2023

Conversation

samcoe
Copy link
Contributor

@samcoe samcoe commented Mar 7, 2023

This PR adds some convenience functionality to auth refresh command. Now if a user logged in with auth login --secure-storage or auth refresh --secure-storage next time they use auth refresh the --secure-storage flag will be implicitly set. Basically, if the token we want to refresh is coming from secure storage we assume that the user wants to continue to use secure storage even if they forgot to add the --secure-storage flag.

@samcoe samcoe requested a review from a team as a code owner March 7, 2023 02:19
@samcoe samcoe self-assigned this Mar 7, 2023
@samcoe samcoe requested review from mislav and removed request for a team March 7, 2023 02:19
@cliAutomation cliAutomation added this to Needs review 🤔 in The GitHub CLI Mar 7, 2023
Copy link
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good; thanks.

Would we also consider supporting explicitly disabling via --secure-storage=false to quit storing in keyring and write the token to config file instead?

The GitHub CLI automation moved this from Needs review 🤔 to Needs to be merged 🎉 Mar 7, 2023
@samcoe
Copy link
Contributor Author

samcoe commented Mar 7, 2023

@mislav I wasn't not planning on supporting that flow. If the user wants to quit storing the token in the keyring using auth logout and then auth login would be the recommended path. I can see the value of just being able to do it in auth refresh but with the release looming I would suggest we hold off and perhaps add it before the next release.

@samcoe samcoe merged commit b74ba55 into trunk Mar 7, 2023
The GitHub CLI automation moved this from Needs to be merged 🎉 to Pending Release 🥚 Mar 7, 2023
@samcoe samcoe deleted the auth-refresh-secure-storage branch March 7, 2023 23:19
@github-actions github-actions bot moved this from Pending Release 🥚 to Done 💤 in The GitHub CLI Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
The GitHub CLI
  
Done 💤
Development

Successfully merging this pull request may close these issues.

None yet

2 participants