implemented create from manifest action

[dbaur]

Description

This pull requests implements the following API methods:

GET /apps/{app_slug}: https://docs.github.com/en/rest/apps/apps?apiVersion=2022-11-28#get-an-app
POST /app-manifests/{code}/conversions: https://docs.github.com/en/rest/apps/apps?apiVersion=2022-11-28#create-a-github-app-from-a-manifest

Fixes #1641

Before submitting a PR:

• Changes must not break binary backwards compatibility. If you are unclear on how to make the change you think is needed while maintaining backward compatibility, CONTRIBUTING.md for details.
• [ x Add JavaDocs and other comments as appropriate. Consider including links in comments to relevant documentation on https://docs.github.com/en/rest .
• Add tests that cover any added or changed code. This generally requires capturing snapshot test data. See CONTRIBUTING.md for details.
• Run mvn -D enable-ci clean install site locally. If this command doesn't succeed, your change will not pass CI.
• Push your changes to a branch other than main. You will create your PR from that branch.

When creating a PR:

• Fill in the "Description" above with clear summary of the changes. This includes:
  • If this PR fixes one or more issues, include "Fixes #" lines for each issue.
  • Provide links to relevant documentation on https://docs.github.com/en/rest where possible.
• All lines of new code should be covered by tests as reported by code coverage. Any lines that are not covered must have PR comments explaining why they cannot be covered. For example, "Reaching this particular exception is hard and is not a particular common scenario."
• Enable "Allow edits from maintainers".

[dbaur]

Currently I have tested the given implementation manually. Before the implementation of automatic tests I would like to clarify two questions with respect to the implementation of the test cases:

• the POST /app-manifests/{code}/conversions is hard to test automatically as it requires a manual interaction that approves the App installation (see documentation for Manifest Flow linked above). While I can record the API interaction for automated testing, the {code} part of the POST request is only short-lived, so if anyone runs the tests against the actual GitHub API, the tests will fail. Is this ok?
• while I would like to implement the tests for GET /apps/{app_slug} in the existing GHAppTest class, the current class and the abstract implementation it inherits from focus solely on tests with respect to authorization as App Installation, where the implementation of the above method does not fit. Should I create a new Test class for those methods?

Best,

Daniel

[bitwiseman]

Currently I have tested the given implementation manually. Before the implementation of automatic tests I would like to clarify two questions with respect to the implementation of the test cases:

• the POST /app-manifests/{code}/conversions is hard to test automatically as it requires a manual interaction that approves the App installation (see documentation for Manifest Flow linked above). While I can record the API interaction for automated testing, the {code} part of the POST request is only short-lived, so if anyone runs the tests against the actual GitHub API, the tests will fail. Is this ok?

Yes, we have some tests that are recorded and never get run against live site for this same reason. Record your data, modify it as needed then add snapshotNotAllowed() to your test method to make it clear.

• while I would like to implement the tests for GET /apps/{app_slug} in the existing GHAppTest class, the current class and the abstract implementation it inherits from focus solely on tests with respect to authorization as App Installation, where the implementation of the above method does not fit. Should I create a new Test class for those methods?

Do what you think is best, just include comments to the code explaining the choice. I'm not that picky about how the testing gets done.

Thanks for contributing.

[dbaur]

Hi,

I have implemented the Tests and stored the API snapshots and removed the draft flag from the PR.

Since the manifest flow test validates the client secret and the pem of the GitHub App, they expose the secrets and the pem to the public internet. I have therefore rotated those secrets in the App itself, however, external scanners will still detect those secrets and flag the contents as insecure.

We can either accept this as it poses no threat (secrets are rotated, app has no installations), or we modify the stored snapshots to some random string that is no longer detected by the secret scanners.

Best,

Daniel

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 🎉

Comparison is base (77e2d8f) 79.89% compared to head (cc23f20) 79.92%.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1647      +/-   ##
============================================
+ Coverage     79.89%   79.92%   +0.02%     
- Complexity     2197     2204       +7     
============================================
  Files           210      211       +1     
  Lines          6671     6680       +9     
  Branches        364      364              
============================================
+ Hits           5330     5339       +9     
  Misses         1127     1127              
  Partials        214      214              

Impacted Files	Coverage Δ
...ain/java/org/kohsuke/github/GHAppFromManifest.java	100.00% <100.00%> (ø)
src/main/java/org/kohsuke/github/GitHub.java	82.81% <100.00%> (+0.30%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.