-
Notifications
You must be signed in to change notification settings - Fork 724
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
Enable fetch plan for current app and account #1630
Changes from 1 commit
d02d076
5bbb5c1
585e872
8585f3a
3344530
4f01b91
702e272
ea5867e
3247196
e09bd39
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
package org.kohsuke.github; | ||
|
||
import java.io.IOException; | ||
|
||
// TODO: Auto-generated Javadoc | ||
/** | ||
* Returns the plan associated with current account. | ||
* | ||
* @author Benoit Lacelle | ||
* @see GHMarketplacePlan#listAccounts() | ||
* @see GitHub#listMarketplacePlans() | ||
*/ | ||
public class GHMarketplacePlanForAccountBuilder extends GitHubInteractiveObject { | ||
private final Requester builder; | ||
private final long accountId; | ||
|
||
/** | ||
* Instantiates a new GH marketplace list account builder. | ||
* | ||
* @param root | ||
* the root | ||
* @param accountId | ||
* the account id | ||
*/ | ||
GHMarketplacePlanForAccountBuilder(GitHub root, long accountId) { | ||
super(root); | ||
this.builder = root.createRequest(); | ||
this.accountId = accountId; | ||
} | ||
|
||
/** | ||
* Fetch the plan associated with the account specified on construction. | ||
* <p> | ||
* GitHub Apps must use a JWT to access this endpoint. | ||
* | ||
* @return a GHMarketplaceAccountPlan | ||
* @throws IOException | ||
* on error | ||
*/ | ||
public GHMarketplaceAccountPlan createRequest() throws IOException { | ||
return builder.withUrlPath(String.format("/marketplace_listing/accounts/%d", this.accountId)) | ||
.fetch(GHMarketplaceAccountPlan.class); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,8 @@ | ||
package org.kohsuke.github; | ||
|
||
import com.nimbusds.jose.JOSEException; | ||
import com.nimbusds.jose.crypto.impl.RSAKeyUtils; | ||
import com.nimbusds.jose.jwk.RSAKey; | ||
import io.jsonwebtoken.Jwts; | ||
import org.apache.commons.io.IOUtils; | ||
import org.kohsuke.github.authorization.AuthorizationProvider; | ||
|
@@ -13,17 +16,25 @@ | |
import java.security.KeyFactory; | ||
import java.security.PrivateKey; | ||
import java.security.spec.PKCS8EncodedKeySpec; | ||
import java.text.ParseException; | ||
import java.time.Instant; | ||
import java.time.temporal.ChronoUnit; | ||
import java.util.Base64; | ||
import java.util.Date; | ||
import java.util.List; | ||
import java.util.Set; | ||
|
||
// TODO: Auto-generated Javadoc | ||
/** | ||
* The Class AbstractGHAppInstallationTest. | ||
*/ | ||
public class AbstractGHAppInstallationTest extends AbstractGitHubWireMockTest { | ||
|
||
private static String ENV_GITHUB_APP_ID = "GITHUB_APP_ID"; | ||
blacelle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private static String ENV_GITHUB_APP_TOKEN = "GITHUB_APP_TOKEN"; | ||
private static String ENV_GITHUB_APP_ORG = "GITHUB_APP_ORG"; | ||
private static String ENV_GITHUB_APP_REPO = "GITHUB_APP_REPO"; | ||
|
||
private static String TEST_APP_ID_1 = "82994"; | ||
private static String TEST_APP_ID_2 = "83009"; | ||
private static String TEST_APP_ID_3 = "89368"; | ||
|
@@ -44,17 +55,31 @@ public class AbstractGHAppInstallationTest extends AbstractGitHubWireMockTest { | |
* Instantiates a new abstract GH app installation test. | ||
*/ | ||
protected AbstractGHAppInstallationTest() { | ||
String appId = System.getenv(ENV_GITHUB_APP_ID); | ||
String appToken = System.getenv(ENV_GITHUB_APP_TOKEN); | ||
try { | ||
jwtProvider1 = new JWTTokenProvider(TEST_APP_ID_1, | ||
new File(this.getClass().getResource(PRIVATE_KEY_FILE_APP_1).getFile())); | ||
jwtProvider2 = new JWTTokenProvider(TEST_APP_ID_2, | ||
new File(this.getClass().getResource(PRIVATE_KEY_FILE_APP_2).getFile()).toPath()); | ||
jwtProvider3 = new JWTTokenProvider(TEST_APP_ID_3, | ||
new String( | ||
Files.readAllBytes( | ||
new File(this.getClass().getResource(PRIVATE_KEY_FILE_APP_3).getFile()).toPath()), | ||
StandardCharsets.UTF_8)); | ||
} catch (GeneralSecurityException | IOException e) { | ||
if (appId != null && appToken != null) { | ||
RSAKey rsaJWK; | ||
try { | ||
rsaJWK = RSAKey.parse(appToken); | ||
} catch (IllegalStateException | ParseException e) { | ||
throw new IllegalStateException("Issue parsing privateKey", e); | ||
} | ||
|
||
jwtProvider1 = new JWTTokenProvider(appId, RSAKeyUtils.toRSAPrivateKey(rsaJWK)); | ||
jwtProvider2 = new JWTTokenProvider(appId, RSAKeyUtils.toRSAPrivateKey(rsaJWK)); | ||
jwtProvider3 = new JWTTokenProvider(appId, RSAKeyUtils.toRSAPrivateKey(rsaJWK)); | ||
} else { | ||
jwtProvider1 = new JWTTokenProvider(TEST_APP_ID_1, | ||
new File(this.getClass().getResource(PRIVATE_KEY_FILE_APP_1).getFile())); | ||
jwtProvider2 = new JWTTokenProvider(TEST_APP_ID_2, | ||
new File(this.getClass().getResource(PRIVATE_KEY_FILE_APP_2).getFile()).toPath()); | ||
jwtProvider3 = new JWTTokenProvider(TEST_APP_ID_3, | ||
new String(Files.readAllBytes( | ||
new File(this.getClass().getResource(PRIVATE_KEY_FILE_APP_3).getFile()).toPath()), | ||
StandardCharsets.UTF_8)); | ||
} | ||
} catch (GeneralSecurityException | IOException | JOSEException e) { | ||
throw new RuntimeException("These should never fail", e); | ||
} | ||
} | ||
|
@@ -89,17 +114,28 @@ private String createJwtToken(String keyFileResouceName, String appId) { | |
* Signals that an I/O exception has occurred. | ||
*/ | ||
protected GHAppInstallation getAppInstallationWithToken(String jwtToken) throws IOException { | ||
if (jwtToken.startsWith("Bearer ")) { | ||
jwtToken = jwtToken.substring("Bearer ".length()); | ||
} | ||
|
||
GitHub gitHub = getGitHubBuilder().withJwtToken(jwtToken) | ||
.withEndpoint(mockGitHub.apiServer().baseUrl()) | ||
.build(); | ||
|
||
GHAppInstallation appInstallation = gitHub.getApp() | ||
.listInstallations() | ||
.toList() | ||
.stream() | ||
.filter(it -> it.getAccount().login.equals("hub4j-test-org")) | ||
.findFirst() | ||
.get(); | ||
GHApp app = gitHub.getApp(); | ||
|
||
GHAppInstallation appInstallation; | ||
if (Set.of(TEST_APP_ID_1, TEST_APP_ID_2, TEST_APP_ID_3).contains(Long.toString(app.getId()))) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This switch leads to invalid recordings: I've done it to prevent recording the N installations of my real-world app. However, when re-running the tests without my custom JWK, the test will list all installations, which will match no stub (as we recorded the other branch of the This remain useful, however one has to edit the recordings manually (e.g. copy-pasting the installations listing from another test). @bitwiseman Would you mind granting me access to the test repo/app ? It may be useful on related matters not requiring markplace-listing. |
||
List<GHAppInstallation> installations = app.listInstallations().toList(); | ||
appInstallation = installations.stream() | ||
.filter(it -> it.getAccount().login.equals("hub4j-test-org")) | ||
.findFirst() | ||
.get(); | ||
} else { | ||
// We may be processing a custom JWK, for a custom GHApp: fetch a relevant repository dynamically | ||
appInstallation = app.getInstallationByRepository(System.getenv(ENV_GITHUB_APP_ORG), | ||
System.getenv(ENV_GITHUB_APP_REPO)); | ||
} | ||
|
||
// TODO: this is odd | ||
// appInstallation | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
{ | ||
"id": 65550, | ||
"slug": "cleanthat", | ||
"node_id": "MDM6QXBwNjU1NTA=", | ||
"owner": { | ||
"login": "solven-eu", | ||
"id": 34552197, | ||
"node_id": "MDEyOk9yZ2FuaXphdGlvbjM0NTUyMTk3", | ||
"avatar_url": "https://avatars.githubusercontent.com/u/34552197?v=4", | ||
"gravatar_id": "", | ||
"url": "https://api.github.com/users/solven-eu", | ||
"html_url": "https://github.com/solven-eu", | ||
"followers_url": "https://api.github.com/users/solven-eu/followers", | ||
"following_url": "https://api.github.com/users/solven-eu/following{/other_user}", | ||
"gists_url": "https://api.github.com/users/solven-eu/gists{/gist_id}", | ||
"starred_url": "https://api.github.com/users/solven-eu/starred{/owner}{/repo}", | ||
"subscriptions_url": "https://api.github.com/users/solven-eu/subscriptions", | ||
"organizations_url": "https://api.github.com/users/solven-eu/orgs", | ||
"repos_url": "https://api.github.com/users/solven-eu/repos", | ||
"events_url": "https://api.github.com/users/solven-eu/events{/privacy}", | ||
"received_events_url": "https://api.github.com/users/solven-eu/received_events", | ||
"type": "Organization", | ||
"site_admin": false | ||
}, | ||
"name": "CleanThat", | ||
"description": "Cleanthat cleans branches automatically to fix/improve your code.\r\n\r\nFeatures :\r\n- Fix branches a pull_requests head\r\n- Open pull_request to fix protected branches\r\n- Format `.md`, `.java`, `.scala`, `.json`, `.yaml` with the help of [Spotless](https://github.com/diffplug/spotless)\r\n- Refactor `.java` files to improve code-style, security and stability", | ||
"external_url": "https://github.com/solven-eu/cleanthat", | ||
"html_url": "https://github.com/apps/cleanthat", | ||
"created_at": "2020-05-19T13:45:43Z", | ||
"updated_at": "2023-01-27T06:10:21Z", | ||
"permissions": { | ||
"checks": "write", | ||
"contents": "write", | ||
"metadata": "read", | ||
"pull_requests": "write" | ||
}, | ||
"events": [ | ||
"pull_request", | ||
"push" | ||
], | ||
"installations_count": 280 | ||
} |
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.
Used to facilitate loading a custom private JWK for a GHApp
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 there an other option aside from adding a new test dependency? How much work does this save?
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.
I considered relying on jjwt, but the feature does not exist yet into there, and people points back to nimbus-jose: jwtk/jjwt#236 (comment)
I would have to go through the JWK to PEM procedure, which is a burden given my own setup (i.e. my app using this lib relies on a JWK, not a PEM). Also, it would be more error-prone to rely on a given PEM content through environment variable (which is again my setup, and how I suggest to manage custom privateKey here).
I tried copying some classes from Nimbus to fill the gap, but it seems to require quite a bunch of classes. I'm having another try but focusing on the minimal set of methods to get a
PrivateKey
.My only justification for this additional lib is existing tests for GitHub App APIs has not dynamic mechanisms for now (neither from local file, or from Env). I may then just drop this code, and be potentially discouraged to contribute here (this is a weak argument as I would be happy to fit into any existing mechanism, the current option would be to copy my own privateKey as a test resources, with a risk of pushing it publicly).
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.
I'm dropping nimbus, with a mechanism to load a PEM from a
Path
provided as environment variable (similarly to the pushed code). However, I will not be a user of this mechanism. It will pave the way for next contribution requiring a custom JWK.