-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
docs: Provide easy alternative to create App JWT token #2937
Conversation
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.
Thanks for the contribution, this is great! I would rather encourage people to use system libraries. I had a couple quick questions and a suggestion before shipping.
docs/github-apps.md
Outdated
In order to create the token, you can create it manually using the following snippet. | ||
|
||
``` csharp | ||
var rsaPrivateKey = "..."; // RSA private key from the App configuration page |
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.
Is this the key itself or the path to the key? I think this could be made more clear.
var rsaPrivateKey = "..."; // RSA private key from the App configuration page | ||
var appId = 1; // The GitHub App Id | ||
|
||
using var rsa = RSA.Create(); |
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.
What's the advantage of the using
statement here?
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.
Since the RSA
object implements the IDisposable
interface, its usually required to invoke the .Dispose()
when the object leaves scope and is no longer needed. There could be some resources not managed by the runtime that needs cleaning up. I haven't decompiled the code to see what's actually there. The syntax for declaring using
without braces was made available in .NET 8.
@kfcampbell do. I added a code comment regarding the required using statements as well and commented on the |
@rasmus first, thank you for this. This token is one of the most confusing parts of getting started with GitHub Apps. I noticed an announcement on the GitHub blog directing GitHub Apps to starting using the From the announcement:
It might be worth updating the sample to use the |
Thanks for following up on this @colbylwilliams, you're correct about the shift. We've made this move in the newly generated SDKs as well. |
This adds a simple code snippet for creating the JWT token required to authenticate GitHub Apps. All dependencies live in the
System
namespace.System.IdentityModel.Tokens.Jwt
System.Security.Claims
System.Security.Cryptography
Before the change?
Developers would think that the only alternative was to import another dependency.
After the change?
Developers are able to create JWT tokens for their GitHub Apps using dependencies they likely already have imported.
Pull request checklist
Tests for the changes have been added (for bug fixes / features)Does this introduce a breaking change?