XAMPPRocky / octocrab

A modern, extensible GitHub API Client for Rust.
Other
1.06k stars 258 forks source link

Listed example for github app auth doesn't work but another provided method does #576

Open ericandoh opened 6 months ago

ericandoh commented 6 months ago

Using the example in https://github.com/XAMPPRocky/octocrab/blob/main/examples/github_app_authentication_manual.rs the below code does not work, returns Failure from github: Error getting octocrab, GitHub: A JSON web token could not be decoded Documentation URL: https://docs.github.com/rest

  // installation is gotten from octocrab as well earlier
  async fn get_octocrab(&self, installation: Installation) -> Result<Octocrab, ScanError> {
        let create_access_token = octocrab::params::apps::CreateInstallationAccessToken::default();
        println!("Processing installation: {}", installation.id.0);

        let access: octocrab::models::InstallationToken = self
            .octocrab
            .post(
                installation.access_tokens_url.as_ref().ok_or_else(|| {
                    ScanError::Github(
                        "Access token URL not found:".into(),
                        installation.id.to_string(),
                    )
                })?,
                Some(&create_access_token),
            )
            .await
            .map_err(|e| ScanError::Github("Errors at this call".into(), e.to_string()))?;
        //...Below code doesnt execute, error in former call
        octocrab::OctocrabBuilder::new()
            .personal_token(access.token)
            .build()
            .map_err(|e| {
                ScanError::Github(
                    "Failed to create octocrab instance with access token".into(),
                    e.to_string(),
                )
            })

However, this code works though (I can do actions on behalf of the app after), so I know my self.octocrab is set up with the correct app ID + secret.

  async fn get_octocrab(&self, installation: Installation) -> Result<Octocrab, ScanError> {
        println!("Processing installation: {}", installation.id.0);

        let (myself, _secret) = self
            .octocrab
            .installation_and_token(installation.id)
            .await
            .map_err(|e| ScanError::Github("I'm an error 2".into(), e.to_string()))?;
        Ok(myself)

any idea why the below works but not the listed example?

ericandoh commented 6 months ago

my octocrab:

octocrab = { version = "^0.33.4", features = ["stream"] }

ztj commented 6 months ago

I believe this and a bunch of other problems were introduced by this pr: https://github.com/XAMPPRocky/octocrab/pull/562

The idea being that if the request isn't going to GitHub then the authorization header shouldn't be sent. The problem is the logic used to make that determination is flawed. It supposed that if there is no authority in the URI then it isn't going elsewhere, which is true, but the wrong assumption is that if there is an authority than it's not going to GitHub. However, in every single URL returned from GitHub APIs, including things like the above installation.access_tokens_url or even the next page URLs for paging, the URL is a complete URL which means it has the authority https://api.github.com or whatever it is.

I've run into this since the version that included the linked PR, it caused me to hit a similar problem as you and it broke my paging code. Whats worse is it often results in weird errors that are misleading, because, GitHub likes to do things like 404 on authorization failures, or tell you about how it failed to parse a JWT (but fails to make the point that it was due to there not being one at all).

You can test my theory by taking the installation.access_tokens_url and making a new URL from it that lacks the authority, basically just keep the path and query part.

I believe the fix will be to essentially complete the work in #562 such that it doesn't break for actual valid API URLs even when they have the authority.