-
Notifications
You must be signed in to change notification settings - Fork 81
Adds a new section in README to guide developers on handling OAuth Client IDs and secrets in CLI/headless apps #87
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
Conversation
|
Hey @williammartin, mind taking a quick look at this when you're free? Appreciate your feedback! |
|
Thank you for the contribution, @kumarmunish! ❤️ Will asked me to follow up on this review. |
andyfeller
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for creating the momentum to close out #1, @kumarmunish! 🫶 After careful consideration, I strongly believe this guidance needed a different approach.
Application developers should understand their security needs and choose the appropriate OAuth authentication flow. As such, the GitHub CLI maintainers should be cautious about making a blanket statement that all CLIs and headless applications follow the same approach we have.
To that effect, I've updated the content instead to explain the circumstances and considerations for why the GitHub CLI manages its client identification that way it does. Hopefully this and information from the original OAuth IETF specification will help other developers make an appropriate decision for themselves.
| ## Managing OAuth Client Identifier | ||
|
|
||
| CLIs or headless applications using [device flow][gh-device] are considered [public clients][rfc-client-types]: | ||
|
|
||
| > _Clients incapable of maintaining the confidentiality of their credentials (e.g., clients executing on the device used by the resource owner, such as an installed native application or a web browser-based application), and incapable of secure client authentication via any other means._ | ||
| For this reason, the GitHub CLI has its OAuth client information committed to `cli/cli` source code as: | ||
|
|
||
| 1. `gh` releases are publicly distributed and can be decompiled to retrieve this information | ||
| 2. `gh` has semi-officially supported `go install` installation as described in [cli/cli#9912](https://github.com/cli/cli/issues/9912) | ||
|
|
||
| Applications using [web application flow][gh-web] must keep the OAuth client secret confidential. | ||
|
|
||
| For more information, see [cli/oauth#1](https://github.com/cli/oauth/issues/1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: I think it is important for GitHub to be more conservative in the guidance here, focusing on why gh is being managed the way it is.
- context: use the real oauth credentials cli#492 demonstrates that publicly releasing artifacts that can be decompiled means the GitHub CLI cannot keep client secret confidential
- https://github.com/github/cli/issues/34 is when
ghmoved from web flow to device flow
Again, the GitHub CLI maintainers cannot speak to all other CLI and headless application use cases, only the decisions we've made the reasoning for it.
BagToad
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andyfeller LGTM and makes sense. Thanks for shaping this up 🙏
|
Thanks a lot for refining the docs, @andyfeller 🙌 |
|
@kumarmunish : after internal discussion with the GitHub Apps team, we've decided to update GitHub Docs "Secure your app's credentials" for the necessary guidance in a central place used by all GitHub users. thank you again for taking the time opening up this PR! |
Fixes #1
📄 Summary
This PR adds a new section to the
README.mdtitled "OAuth Guidance for CLI and Headless Apps", providing clear guidance on handling OAuth Client ID and Secret in public clients such as CLI tools.It incorporates recommendations made by maintainers in Issue #1, highlighting:
📝 Changes
README.mdwith practical guidance for OAuth in public clients🔍 Related Issues
Fixes or completes: #1
✅ Checklist
🙋♂️ Notes for Reviewers
The goal here is to formalize guidance that was already shared in Issue #1 but never documented. Feedback welcome on tone or placement within the README.