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 token support, updated docs #35

Merged
merged 1 commit into from
Dec 12, 2022
Merged

Conversation

andyfeller
Copy link
Contributor

fixes #33
fixes #34

This commit is a bit of a crude workaround for usage where the token is a server-to-server GitHub App token for higher rate limits and fixing some documentation typos and missing information.

Unlike APIs for authenticated user PATs, the GitHub App installation tokens doesn't have the same capabilities, so this PR bypasses some of the checks previously used for testing if the PAT has the necessary permissions and simply does the job.

Along with that, this PR fixes documentation as --ghe-url was an invalid flag and our docs didn't explain what to do with the results.

fixes #33
fixes #34

This commit is a bit of a crude workaround for usage where the token is a server-to-server GitHub App token for higher rate limits and fixing some documentation typos and missing information.
@andyfeller andyfeller self-assigned this Nov 4, 2022
@andyfeller andyfeller requested a review from a team as a code owner November 4, 2022 20:44
@bryantson
Copy link
Collaborator

Okay with --ghe-url fix as that is a right fix. Need a further review and test on presenting an alternative option on using GitHub App token since there is no additional check around GitHub App as it is for user token.

@andyfeller
Copy link
Contributor Author

Okay with --ghe-url fix as that is a right fix. Need a further review and test on presenting an alternative option on using GitHub App token since there is no additional check around GitHub App as it is for user token.

@bryantson : thanks for following up!

If you're saying that we should check the GitHub App installation access token's permissions within the organization, I'm 80% of how to do that with a small caveat:

  • List repositories accessible to the app installation should give you information about repositories the token has access to, however this doesn't use an installation token but a JWT

  • List app installations for an organization gives information about GitHub App installations, which technically has the permissions, but it lists all the installations of the app and requires you to know which installation is associated with the token you're using:

    {
      "total_count": 1,
      "installations": [
        {
          "id": 25381,
          "account": {
            "login": "octo-org",
            "id": 6811672,
            "node_id": "MDEyOk9yZ2FuaXphdGlvbjY4MTE2NzI=",
            "avatar_url": "https://avatars3.githubusercontent.com/u/6811672?v=4",
            "gravatar_id": "",
            "url": "https://api.github.com/users/octo-org",
            "html_url": "https://github.com/octo-org",
            "followers_url": "https://api.github.com/users/octo-org/followers",
            "following_url": "https://api.github.com/users/octo-org/following{/other_user}",
            "gists_url": "https://api.github.com/users/octo-org/gists{/gist_id}",
            "starred_url": "https://api.github.com/users/octo-org/starred{/owner}{/repo}",
            "subscriptions_url": "https://api.github.com/users/octo-org/subscriptions",
            "organizations_url": "https://api.github.com/users/octo-org/orgs",
            "repos_url": "https://api.github.com/users/octo-org/repos",
            "events_url": "https://api.github.com/users/octo-org/events{/privacy}",
            "received_events_url": "https://api.github.com/users/octo-org/received_events",
            "type": "Organization",
            "site_admin": false
          },
          "repository_selection": "selected",
          "access_tokens_url": "https://api.github.com/app/installations/25381/access_tokens",
          "repositories_url": "https://api.github.com/installation/repositories",
          "html_url": "https://github.com/organizations/octo-org/settings/installations/25381",
          "app_id": 2218,
          "target_id": 6811672,
          "target_type": "Organization",
          "permissions": {
            "deployments": "write",
            "metadata": "read",
            "pull_requests": "read",
            "statuses": "read"
          },
          "events": [
            "deployment",
            "deployment_status"
          ],
          "created_at": "2017-05-16T08:47:09.000-07:00",
          "updated_at": "2017-06-06T11:23:23.000-07:00",
          "single_file_name": "config.yml",
          "has_multiple_single_files": true,
          "single_file_paths": [
            "config.yml",
            ".github/issue_TEMPLATE.md"
          ],
          "app_slug": "github-actions",
          "suspended_at": null,
          "suspended_by": null
        }
      ]
    }

If you have any code on how CheckAdminRights could be refactored, I'd appreciate it.

If this is a bit difficult or something that a bit of work needs to be figured out how to do it, then I'd like to suggest creating a follow up issue to dig into that so permission checking can be enhanced.

Thoughts?

@andyfeller
Copy link
Contributor Author

Manual testing notes

  1. Installed app with read-only permissions as documented

    Screen Shot 2022-11-06 at 12 32 30 AM

  2. Generated GitHub App installation token

    bash-5.2$ gh token generate --key ~/Downloads/gh-repo-stats-demo.2022-11-04.private-key.pem --app_id ####### --installation_id #######
    {
      "token": "ghs_XXXXXXXXXXXXXXXXX",
      "expires_at": "2022-11-06T05:28:41Z"
    }
  3. Setup token and ran gh-repo-stats for organization where GitHub App is installed

    bash-5.2$ export GITHUB_TOKEN="ghs_XXXXXXXXXXXXXXXXX"
    bash-5.2$ ./gh-repo-stats --token-type app -o tinyfists
    
    ######################################################
    ######################################################
    ############# GitHub repo list and sizer #############
    ######################################################
    ######################################################
    
    Skip checking user PAT admin rights for GitHub App token
    ------------------------------------------------------
    Getting repositories for org: tinyfists
    [5000] API attempts remaining...
    Analyzing Repo: publish-packages-to-repo-demo
    Analyzing Repo: issue-driven-github-admin
    Analyzing Repo: issue-form-preview
    Analyzing Repo: stacks-experiment
    Analyzing Repo: stacks-experiment-01
    Analyzing Repo: services
    Analyzing Repo: githubcustomer
    Analyzing Repo: multi-runner-poc
    Analyzing Repo: actions-experiments
    Analyzing Repo: git-xargs-1
    Analyzing Repo: git-xargs-2
    Analyzing Repo: git-xargs-3
    Analyzing Repo: git-xargs
    Analyzing Repo: spring-framework
    Gathered all repositories for org: tinyfists
    
    ######################################################
    The script has completed
    
    Results file:[tinyfists-all_repos-202211060026.csv]
    ######################################################
  4. bash-5.2$ cat tinyfists-all_repos-202211060026.csv
    Org_Name,Repo_Name,Is_Empty,Last_Push,Last_Update,isFork,Repo_Size(mb),Record_Count,Collaborator_Count,Protected_Branch_Count,PR_Review_Count,Milestone_Count,Issue_Count,PR_Count,PR_Review_Comment_Count,Commit_Comment_Count,Issue_Comment_Count,Issue_Event_Count,Release_Count,Project_Count,Full_URL,Migration_Issue
    tinyfists,publish-packages-to-repo-demo,false,2021-10-11T19:39:33Z,2021-10-11T19:39:32Z,false,0,23,18,0,0,0,0,2,0,0,0,3,0,0,https://github.com/tinyfists/publish-packages-to-repo-demo,FALSE
    tinyfists,issue-driven-github-admin,false,2022-10-11T03:24:34Z,2022-10-14T22:03:38Z,false,2,1628,37,2,1,0,138,3,1,0,891,544,10,1,https://github.com/tinyfists/issue-driven-github-admin,FALSE
    tinyfists,issue-form-preview,false,2021-11-01T16:21:13Z,2021-11-01T16:21:15Z,false,0,18,18,0,0,0,0,0,0,0,0,0,0,0,https://github.com/tinyfists/issue-form-preview,FALSE
    tinyfists,stacks-experiment,false,2021-11-21T03:07:02Z,2022-09-12T20:33:48Z,false,0,22,18,0,0,0,0,0,0,0,0,0,4,0,https://github.com/tinyfists/stacks-experiment,FALSE
    tinyfists,stacks-experiment-01,false,2021-11-10T19:52:51Z,2021-11-10T19:52:53Z,false,0,19,18,1,0,0,0,0,0,0,0,0,0,0,https://github.com/tinyfists/stacks-experiment-01,FALSE
    tinyfists,services,false,2022-06-29T18:53:16Z,2022-05-10T03:19:38Z,false,0,49,19,1,0,1,9,0,0,0,12,7,0,0,https://github.com/tinyfists/services,FALSE
    tinyfists,githubcustomer,false,2022-06-04T17:00:43Z,2022-05-10T03:05:16Z,false,0,25,18,0,0,0,4,0,0,0,0,3,0,0,https://github.com/tinyfists/githubcustomer,FALSE
    tinyfists,multi-runner-poc,false,2022-08-03T12:49:52Z,2022-08-03T12:44:35Z,false,0,18,18,0,0,0,0,0,0,0,0,0,0,0,https://github.com/tinyfists/multi-runner-poc,FALSE
    tinyfists,actions-experiments,false,2022-10-28T19:58:24Z,2022-10-28T19:38:34Z,false,0,18,18,0,0,0,0,0,0,0,0,0,0,0,https://github.com/tinyfists/actions-experiments,FALSE
    tinyfists,git-xargs-1,false,2022-11-01T03:18:18Z,2022-11-01T03:18:17Z,false,0,18,18,0,0,0,0,0,0,0,0,0,0,0,https://github.com/tinyfists/git-xargs-1,FALSE
    tinyfists,git-xargs-2,false,2022-11-01T03:18:30Z,2022-11-01T03:18:29Z,false,0,18,18,0,0,0,0,0,0,0,0,0,0,0,https://github.com/tinyfists/git-xargs-2,FALSE
    tinyfists,git-xargs-3,false,2022-11-01T03:18:40Z,2022-11-01T03:18:39Z,false,0,18,18,0,0,0,0,0,0,0,0,0,0,0,https://github.com/tinyfists/git-xargs-3,FALSE
    tinyfists,git-xargs,false,2022-11-01T03:19:51Z,2022-11-01T03:19:49Z,false,0,18,18,0,0,0,0,0,0,0,0,0,0,0,https://github.com/tinyfists/git-xargs,FALSE
    tinyfists,spring-framework,false,2022-11-03T15:03:00Z,2022-11-03T15:07:15Z,true,177,18,18,0,0,0,0,0,0,0,0,0,0,0,https://github.com/tinyfists/spring-framework,FALSE

@andyfeller
Copy link
Contributor Author

@mona-actions/team-es : looking to bump eyes on this review, help me understand what is reasonable around @bryantson comment on value in checking if permissions are enabled for a GitHub App.

Copy link
Collaborator

@bryantson bryantson left a comment

Choose a reason for hiding this comment

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

Go for it!

@andyfeller andyfeller merged commit 0058f97 into main Dec 12, 2022
@andyfeller andyfeller deleted the issue-34-github-app branch December 12, 2022 21:07
Copy link

@Jhayzhel Jhayzhel left a comment

Choose a reason for hiding this comment

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

#36

@Djnjanetstiers Djnjanetstiers linked an issue May 2, 2023 that may be closed by this pull request
@andyfeller
Copy link
Contributor Author

#36

@Jhayzhel : thank you for raising a concern regarding recent changes in GH CLI and how it has expanded to support secure storage of long lived tokens.

Is there a specific concern regarding the changes introduced in this PR you wanted to discuss regarding the introduction of secure storage?

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