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

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

Open
ericandoh opened this issue Feb 12, 2024 · 2 comments
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@ericandoh
Copy link

ericandoh commented Feb 12, 2024

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 ericandoh changed the title listed example for github app auth doesn't work but another provided method does Listed example for github app auth doesn't work but another provided method does Feb 12, 2024
@ericandoh
Copy link
Author

my octocrab:

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

@XAMPPRocky XAMPPRocky added bug Something isn't working help wanted Extra attention is needed good first issue Good for newcomers labels Feb 13, 2024
@ztj
Copy link
Contributor

ztj commented Feb 29, 2024

I believe this and a bunch of other problems were introduced by this pr: #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants