Skip to content

401 when trying to extract GHUser#type #1816

Closed

Description

@Haarolean

Hi, a code like this

new GitHubBuilder()
        .withJwtToken("token")
        .build()
        .getApp()
        .getInstallationById(githubInstallationId)
        .getAccount()
        .getType()

leads to 401:

Caused by: org.kohsuke.github.HttpException: {"message":"Bad credentials","documentation_url":"https://docs.github.com/rest"}
	at org.kohsuke.github.GitHubConnectorResponseErrorHandler$1.onError(GitHubConnectorResponseErrorHandler.java:62) ~[github-api-unbridged-1.318.jar:na]

There are a few issues here:

  1. The field is already initialized as "Organization" after calling getAccount(), so populating here is not necessary, perhaps.
  2. installation.getAccount().getUrl() returns https://api.github.com/users/<ORGANIZATION_LOGIN>, which perhaps is invalid, because it's an organization, not a user.

Activity

  1. Haarolean commented on Mar 14, 2024

    @Haarolean
    ContributorAuthor

    Unfortunately, I couldn't find a way to work around this by determining installation owner's account type, please let me know if there's something I missed

    UPD: Found a workaround:

    ghInstallation.getRoot().setConnector(HttpConnector.OFFLINE);
    ghInstallation.getAccount().getType();
    
  2. bitwiseman commented on Mar 15, 2024

    @bitwiseman
    Member

    Here's the problem:

    public String getType() throws IOException {
    populate();
    return type;

    populate() should only be called if type is null.

  3. Haarolean commented on Mar 16, 2024

    @Haarolean
    ContributorAuthor

    @bitwiseman got it. How should we solve this taking into consideration the fact that there are multiple fields like this? Should we track the "populated = true/false" flag for an entity or something else?

  4. Haarolean commented on Jul 25, 2024

    @Haarolean
    ContributorAuthor
  5. bitwiseman commented on Aug 5, 2024

    @bitwiseman
    Member

    @Haarolean
    In this specific case, you can guard the populate() call with if (type == null).
    I think some of the classes have a populate(bool doPopulate) method or populate(Object objectToCheckForNullValue) to streamline this process.

    Tracking bool isPopulated; as field could also work.
    I don't like the way we currently don't know what fields we have actually loaded in an instance, but I haven't had time to ponder a better solution for the range partial fields that are returned. Maybe have the base record and then use interfaces to show what fields are present.

    What are your thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions