-
Notifications
You must be signed in to change notification settings - Fork 733
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
Support all available endpoints for Github App with preview request #570
Comments
cc/ @bitwiseman |
I'm trying to create a pull request using installation token, which is expose via a My current workaround is have a method in |
@tkbky |
@bitwiseman |
Hi @tkbky, I am not entirely sure if I understood what your use case is and what is causing the issue. Would you care to share with us some code snippets (stacktraces too if applicable) of how you're trying to do it? |
@bitwiseman Please feel free to cc me on any Github App-related issue...I will be more than happy to help anyone with this feature of the SDK. @tkbky Okay, I think I finally got what is going on....but we need to address a meta-consideration or two before we can discuss a solution.
IMHO, I think you may have misunderstood the docs. GET /:repos/:owner/:repo is GithubApp ready which means it plays nice with GitHub App's functionality I use this on an internal application at work..and this is how it's defined (curated function) as shown below: void postComments(String oAuthToken, AnalysisRequestModel model, String... comments) throws *****Exception {
var gitHub = new GitHubBuilder().withOAuthToken(oAuthToken).build();
var repository = gitHub.getRepository(model.getRepositoryFullname());
var pullRequest = repository.getPullRequest(model.getPullRequestNumber());
for (String comment : comments) {
pullRequest.comment(comment);
}
} Again, If you could share how you're doing it, app's permissions and log we may be able to better assist you.
We need to unpack this: 1 :: Although Github isn't much upfront about it, the 2 :: Creating a method in GHPullRequest class that creates a pull request sounds a bit funny. Given the foundation structure that this sdk has been built upon, it would (again, IMHO) make more sense to use the GHRepository to do it.
My interpretation of this is that "for accessing the GitHub APP-related API" you need to set the media-type to For instance, the code-snippet I shared with you shouldn't work as In case you have an example where this media-type is required just because you're accessing it via GitHub App (which would potentially prove me wrong), I would happily work on a PR to expand this feature's integration with the current SDK's classes. |
Thanks for breaking this down with thorough explanation. Appreciate that. 😃
Yes. While it's enabled for Github App, it has to be a user-to-server request when accessing non-organization repository.
While it's not considered as a workaround, A clear separation between the two would be good, because they are behaving differently in some cases, i.e. the server-to-server, and user-to-server request.
Do agree that my workaround sounds weird 😄. What I'm trying to do is make creating a pull request not depending on a i.e.
That's just my interpretation of the Github API doc, but it doesn't seems like it needs to. 😅 -- After all, the problem that I have becomes: How do I create a pull request using a Github App installation token? |
Hi @tkbky I will take a look at it when I get home today. |
@tkbky Thanks for provided code-snippets, I could reproduce it locally now.
I got now where the source of confusion is. Here is what you should take into consideration: GET /:repos/:owner/:repo is not a user-to-server request When we install a GithubApp on a user account or organisation (Let's call it GHPerson), we have to specify the It's paramount that you understand that We can infer two important things out of that:
This is an example of how to create a PR on a repository which is part of the permission scope of the GithubApp: String jwtToken = createJWT("44435", 600000); //sdk-github-api-app-test
GitHub gitHubApp = new GitHubBuilder().withJwtToken(jwtToken).build();
GHApp app = gitHubApp.getApp();
for (GHAppInstallation appInstallation : app.listInstallations()) {
Map<String, GHPermissionType> permissions = new HashMap<>();
permissions.put("pull_requests", GHPermissionType.WRITE);
GHAppInstallationToken appInstallationToken = appInstallation
.createToken(permissions)
.create();
GitHub githubAuthAsInst = new GitHubBuilder()
.withOAuthToken(appInstallationToken.getToken(), "")
.build();
GHRepository repository = githubAuthAsInst.getRepository("PauloMigAlmeida/github-api");
repository.createPullRequest(
"Test PR", "PauloMigAlmeida-patch-1",
"master", "Test create PR via Github App");
} PR created via GithubApp: PauloMigAlmeida#1 Any out-of-scope repository should return an HTTP status code 403 with the message: Exception in thread "main" org.kohsuke.github.HttpException:
{"message":"Resource not accessible by integration","documentation_url":"https://developer.github.com/v3/pulls/#create-a-pull-request"} Having said that, we can safely come to the conclusion that GET /:repos/:owner/:repo is not a user-to-server request and any error obtained while playing with it is related to either a permission issue or it's out-of-scope of the Github App due to its security design.
I see. While a "clear separation" is always a good thing to bear in mind, it would be beneficial if you could consider evaluating a couple of other points too. SDK design: the github-api sdk was designed based on the idea that classes like GitHub gitHubAnonymous = GitHub.connectAnonymously();
GitHub gitHubApp = new GitHubBuilder().withJwtToken(jwtToken).build();
GitHub gitHubAsUser = new GitHubBuilder().withPassword("my_user","my_password").build();
GitHub githubAuthAsInst = new GitHubBuilder().withOAuthToken("appInstallationToken", "").build();
// Should work because we can anonymously get a public repository
GHRepository repository = gitHubAnonymous.getRepository("PauloMigAlmeida/github-api");
// Should work because I installed my github app on my user and allowed it to be accessed
GHRepository repository = githubAuthAsInst.getRepository("PauloMigAlmeida/github-api");
// Should not work because my github app isn't installed on the nfscan org even though this repo is public
GHRepository repository = githubAuthAsInst.getRepository("nfscan/nfscan");
// Should not work as GitHub app (with JWT token) isn't meant to access it
GHRepository repository = gitHubApp.getRepository("PauloMigAlmeida/github-api");
// Should work becuase the repository is public and because I have access to it too
GHRepository repository = gitHubAsUser.getRepository("PauloMigAlmeida/github-api"); So it is up to the developer to diligently pass the right backwards-compatibility: This sdk has the peculiarity of being reasonably old and yet still being relevant to the java community. However, it was conceived when GitHub didn't have many of the features it has today which implies that certain decisions in the past affect our implementation freedom if we want to maintain backwards-compatibility with previous versions of the SDK. Deprecating classes/methods are a way of doing it up to a certain point only. Don't forget about 2 things: My point is given all the design/maintainability principles applied to this sdk so far the easiest way would be to write getting started/usage docs so that other developers can grasp it rapidly rather than redesigning/reimplementing this sdk. I must confess I would love to sit with many of you guys and discuss the 2.0 version of this sdk but this is definitely outside the scope of this PR. Let's catch up on it when you feel like doing it.
I'm preassuming that you created the Last but not least, @bitwiseman Even though this is not impacting anyone using the SDK currently, what do think about creating a method on GithubBuilder that makes people wrap their heads around how to use the GithubApp right away? .... something like:
The internal implementation would be something akin to: public GitHubBuilder withAppToken(String token){
return withOAuthToken(token, "");
} While this is just for readability/cosmetic purposes, this can also reduce future misunderstandings with the usage the sdk. I will defer the final decision to you and in case you are in favour of that, I will happily propose a more elaborated PR on that. @tkbky Happy coding guys :) |
@PauloMigAlmeida Thanks for taking time for all these detailed clarifications and suggestions, appreciate that. |
@PauloMigAlmeida I've been focusing on getting CI working and clearing the backlog of PRs. Open to discussions of ideas for 2.x API but my time is limited. |
Is there any documentation for this @PauloMigAlmeida ? I'm specifically looking for if the SDK has any classes for handling the initial GitHub app authentication to get the installation token: |
@tkbky any time :)
@bitwiseman Great, I'm gonna work on a PR later today which will add that convenience method and also some docs around its usage. @timja I'm working on the documentation piece for this feature to make it easier for future developers. However, answering your question. Yes, the sdk has classes for dealing with GitHub app auth to get the installation token. I had posted this piece of code in this thread but to save you from reading it all, I will post it again :) String jwtToken = createJWT("44435", 600000); //sdk-github-api-app-test
GitHub gitHubApp = new GitHubBuilder().withJwtToken(jwtToken).build();
GHApp app = gitHubApp.getApp();
for (GHAppInstallation appInstallation : app.listInstallations()) {
Map<String, GHPermissionType> permissions = new HashMap<>();
permissions.put("pull_requests", GHPermissionType.WRITE);
GHAppInstallationToken appInstallationToken = appInstallation
.createToken(permissions)
.create();
GitHub githubAuthAsInst = new GitHubBuilder()
.withOAuthToken(appInstallationToken.getToken(), "")
.build();
GHRepository repository = githubAuthAsInst.getRepository("PauloMigAlmeida/github-api");
repository.createPullRequest(
"Test PR", "PauloMigAlmeida-patch-1",
"master", "Test create PR via Github App");
} We decided not to create methods for generating the JWT in the SDK as we felt that this isn't the sdk's value-proposition and everyone should use the JWT generation libraries/methods they see fit. Having said that, <dependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt-api</artifactId>
<version>0.10.5</version>
</dependency>
<dependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt-impl</artifactId>
<version>0.10.5</version>
<scope>runtime</scope>
</dependency>
<dependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt-jackson</artifactId>
<version>0.10.5</version>
<scope>runtime</scope>
</dependency> static PrivateKey get(String filename) throws Exception {
byte[] keyBytes = Files.toByteArray(new File(filename));
PKCS8EncodedKeySpec spec = new PKCS8EncodedKeySpec(keyBytes);
KeyFactory kf = KeyFactory.getInstance("RSA");
return kf.generatePrivate(spec);
}
static String createJWT(String githubAppId, long ttlMillis) throws Exception {
//The JWT signature algorithm we will be using to sign the token
SignatureAlgorithm signatureAlgorithm = SignatureAlgorithm.RS256;
long nowMillis = System.currentTimeMillis();
Date now = new Date(nowMillis);
//We will sign our JWT with our ApiKey secret
Key signingKey = get("sdk-github-api-app-test.2019-10-23.private-key.der");
//Let's set the JWT Claims
JwtBuilder builder = Jwts.builder()
.setIssuedAt(now)
.setIssuer(githubAppId)
.signWith(signingKey, signatureAlgorithm);
//if it has been specified, let's add the expiration
if (ttlMillis > 0) {
long expMillis = nowMillis + ttlMillis;
Date exp = new Date(expMillis);
builder.setExpiration(exp);
}
//Builds the JWT and serializes it to a compact, URL-safe string
return builder.compact();
} PS.: In order to generate a JWT you must use the signing key that your Github app gives you. The problem is that Java doesn't play nice with To convert the Github App pem key I did: openssl pkcs8 -topk8 -inform PEM -outform DER -in sdk-github-api-app-test.2019-10-23.private-key.pem -out sdk-github-api-app-test.2019-10-23.private-key.der -nocrypt Hope that this helps you. |
Thanks a lot! Another question, there’s nothing in the SDK for automatically generating a new installation token when the old one expires right? I couldn’t see something like a supplier or function that could be passed to generate the oauth token |
That's pretty cool! Looking forward to seeing this implemented :)
Not really. This SDK works as a stateless wrapper of the GitHub endpoints which means that we don't store any of those generated tokens. It's up to the developer to decide when to call We do give developers access to the |
Sorry to hijack the thread, but I'm very interested by this @timja (and have Jenkins act as a GitHub app to read repos / update status). Let me know if / how I can help. |
I got distracted from this and haven't got back to it recently, I'll try get back to it next week otherwise there's a branch here: It needs docs, tests and adjusting the api it calls to check whether the token is valid, currently it tries to retrieve the current user's details which doesn't work for a github app. It's quite a nice check though as you can show in the UI "authenticated as timja-bot" But the code does work as is, (or at least it worked before I just merged master and bumped to the latest release of github-api) |
@ojacques I finally got back to this, there's a functional PR here: |
A side-note to this thread which has been very helpful to my own case: if you want to open a PR over a private repository, in order to get the GHRepository instance, you need as permissions in the installationToken:
Quite interestingly, without the READ permission over "metadata", you can still access the GHRepository instance through |
Not able to access all available endpoints with Github App. Only certain endpoints that are accessible because they have this custom media type set internally.
According to this doc, accessing the Github API with Github App requires custom media type
application/vnd.github.machine-man-preview+json
. Currently, there isn't a way to set this.The text was updated successfully, but these errors were encountered: