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

GitHub app: isCredentialValid doesn't work #582

Closed
timja opened this issue Oct 30, 2019 · 8 comments · Fixed by #764
Closed

GitHub app: isCredentialValid doesn't work #582

timja opened this issue Oct 30, 2019 · 8 comments · Fixed by #764

Comments

@timja
Copy link
Collaborator

timja commented Oct 30, 2019

I'm trying to add GitHub app support to the github-branch-source Jenkins plugin,
I'm facing 2 main issues,

  1. From the GitHub class I can't tell if I'm using a GitHub app

  2. isCredentialValid / getMyself don't work

org.kohsuke.github.GHFileNotFoundException: {"message":"Resource not accessible by integration","documentation_url":"https://developer.github.com/v3/users/#get-the-authenticated-user"}

Ideally isCredentialValid would do something else when using a GitHub app and not sure if there's anything that can be done about getMyself

cc @PauloMigAlmeida

@PauloMigAlmeida
Copy link
Contributor

PauloMigAlmeida commented Oct 30, 2019

Hi @timja

Both points have been discussed on #570. I really suggest you read it (I know it's bit lengthy, but that's going to answer most questions you may have)

From the GitHub class I can't tell if I'm using a GitHub app

The GitHub class should be diligently created using the appropriate credentials/permissions for what you need... which means that if you get an instance of the GitHub class using new GitHubBuilder().withJwtToken(jwtToken).build() then you know you are 'using a Github App'.

isValidCredential / getMyself don't work

org.kohsuke.github.GHFileNotFoundException: {"message":"Resource not accessible by integration","documentation_url":"https://developer.github.com/v3/users/#get-the-authenticated-user"}

This was also discussed on #570 and I proposed to @bitwiseman that a convenience/cosmetic method was created to make it easier for developers to know how to make heads or tails of the multiple ways of authenticating to GIthub with this library.

Basically, the summarised version of why you are facing it would be:

  1. App Installation Token is an OAuth token (so is personal access tokens and oauth apps)
  2. This library defined the behaviour (many years ago) that if you pass an oauth token with no login then it tries to call the getMyself method which for App Installation token flow isn't applicable as /user isn't a GitHub App-ready endpoint. Since we may have users relying on this behaviour, we won't change it until we are ready to bump to the 2.x version of this sdk.

How to address it:

Before the #583 PR is approved:

GitHub githubAuthAsInst = new GitHubBuilder()
                    .withOAuthToken(appInstallationToken.getToken(), "")
                    .build();

After the #583 PR is approved:

GitHub githubAuthAsInst = new GitHubBuilder()
                    .withAppInstallationToken(appInstallationToken.getToken())
                    .build();

Hope this helps

@timja
Copy link
Collaborator Author

timja commented Oct 30, 2019

@PauloMigAlmeida thanks for the reply I have read it

I can authenticate fine there's no issue with that.
i.e. I have the code working

What doesn't work is some of the validation that's being done by the UI and some background processes.

i.e. the UI tries to check a credential is valid, and the method isCredentialValid doesn't work if you're using a GitHub app.

@PauloMigAlmeida
Copy link
Contributor

@timja I see. Help me understand this

What doesn't work is some of the validation that's being done by the UI and some background processes.
i.e. the UI tries to check a credential is valid, and the method isValidCredential doesn't work if you're using a GitHub app.

My brain must not be working today :( That doesn't sound like it's associated with this sdk but with some Jenkins plugin mechanism. Am I right? Or am I missing something?

@timja
Copy link
Collaborator Author

timja commented Oct 30, 2019

@timja timja changed the title GitHub app: isValidCredential doesn't work GitHub app: isCredentialValid doesn't work Oct 30, 2019
@PauloMigAlmeida
Copy link
Contributor

Hmmm got it now. Thanks for the link.

I see what you are trying to do but the GitHub.isCredentialValid() isn't applicable to Github App as it make a request to /user and this isn't a Github App-ready endpoint. (this would've worked on the "old" oauth apps though)

TBH, the /user is really a swiss knife that has been used to verify whether or not the credentials provided would work....but I don't think there isn't an equivalent one for GIthub Apps as their permissions will vary a lot given the fine-grained permissions users can set.

The closest you can get to it (for now) authenticated as a Github App (whether via JWT or App Installation Token) would be to implement in your source code the same try/catch mechanism used on GitHub.isCredentialValid() but using Github.getRateLimit() as this seems to be an available endpoint for Apps

@bitwiseman let me pick your brains on this one. Do you think we should make GitHub.isCredentialValid() call under-the-hood different GitHub endpoints depending on the credentials used or is it the case that we simply document that developers should only use this method when using oauth apps or username/pass credentials?

@bitwiseman
Copy link
Member

Do you think we should make GitHub.isCredentialValid() call under-the-hood different GitHub endpoints depending on the credentials used or is it the case that we simply document that developers should only use this method when using oauth apps or username/pass credentials?

Hm, maybe we want to add a isAppCredentialValid()? Otherwise people might call this method, get true but then get an error when using methods that require JWT.

@PauloMigAlmeida
Copy link
Contributor

PauloMigAlmeida commented Nov 9, 2019

@bitwiseman that sounds a good idea. I will implement it in the next few days and open a PR.

@timja anything else you experienced that is blocking you from progressing on what you are doing?

@timja
Copy link
Collaborator Author

timja commented Nov 9, 2019

not anymore, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants