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

omitempty tag on InstallationAccessTokenOptions.Repositories masking functionality of GitHub API #3106

Closed
gillisandrew opened this issue Mar 16, 2024 · 11 comments · Fixed by #3155
Assignees

Comments

@gillisandrew
Copy link

When creating an app installation access token through the REST API (POST /app/installations/{installation_id}/access_tokens), if the repositories parameter is set to an empty array, the API returns a list of all accessible repositories. However, the client currently strips empty arrays (omitempty) when marshaling the InstallationAccessTokenOptions.Repositories value making that functionality inaccessible through the client.

go-github/github/apps.go

Lines 45 to 57 in 169ad4d

type InstallationTokenOptions struct {
// The IDs of the repositories that the installation token can access.
// Providing repository IDs restricts the access of an installation token to specific repositories.
RepositoryIDs []int64 `json:"repository_ids,omitempty"`
// The names of the repositories that the installation token can access.
// Providing repository names restricts the access of an installation token to specific repositories.
Repositories []string `json:"repositories,omitempty"`
// The permissions granted to the access token.
// The permissions object includes the permission names and their access type.
Permissions *InstallationPermissions `json:"permissions,omitempty"`
}

@gmlewis
Copy link
Collaborator

gmlewis commented Mar 16, 2024

According to the official docs:

Optionally, you can use the repositories or repository_ids body parameters to specify individual repositories that the installation access token can access. If you don't use repositories or repository_ids to grant access to specific repositories, the installation access token will have access to all repositories that the installation was granted access to. The installation access token cannot be granted access to repositories that the installation was not granted access to. Up to 500 repositories can be listed in this manner.

so these fields are optional, hence the omitempty, and it's not clear (to me) what the behavior is if you were to send it an empty array (as opposed to not sending the field at all, which omitempty does).

You didn't state what you are trying to do by providing it an empty array.

Are you trying to remove access to all repositories? If so, I'm wondering if that might be a different endpoint, such as this one?

If you are trying to do something else, then I need to warn you against removing omitempty from optional body parameters, as that has historically wreaked havoc with other uses of the endpoint, and we've had to create additional endpoints to address the other behavior.

So please clarify what you are trying to do, and if you would like to create a PR to address the issue.

Thanks.

@gillisandrew
Copy link
Author

gillisandrew commented Mar 17, 2024

So the documentation doesn't actually specify this behavior but basically you can have the endpoint return the list of all accessible repositories in the installation by specifying an empty array for the repositories parameter. It saved a round trip to the separate endpoint.

Command with empty array

curl -s "https://api.github.com/app/installations/$APP_INSTALL_ID/access_tokens" \
  -H "Accept: application/vnd.github+json" \
  -H "Authorization: Bearer $APP_JWT" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  -d '{"repositories":[],"permissions":{"contents":"read"}}' 

Output

{
  "token": "<token>",
  "expires_at": "2024-03-17T02:30:32Z",
  "permissions": {
    "contents": "read",
    "metadata": "read"
  },
  "repositories": ["<all repos in installation...>"],
  "repository_selection": "all"
}

Command with null or no repositories

curl -s "https://api.github.com/app/installations/$APP_INSTALL_ID/access_tokens" \
  -H "Accept: application/vnd.github+json" \
  -H "Authorization: Bearer $APP_JWT" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  -d '{"repositories":null,"permissions":{"contents":"read"}}'
# OR
curl -s "https://api.github.com/app/installations/$APP_INSTALL_ID/access_tokens" \
  -H "Accept: application/vnd.github+json" \
  -H "Authorization: Bearer $APP_JWT" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  -d '{"permissions":{"contents":"read"}}'

Output

{
  "token": "<token>",
  "expires_at": "2024-03-17T02:30:32Z",
  "permissions": {
    "contents": "read",
    "metadata": "read"
  },
  "repository_selection": "all"
}

@gmlewis
Copy link
Collaborator

gmlewis commented Mar 17, 2024

Awesome... Thank you for the detailed example!

Then I'm thinking we need to have a new, separate endpoint that handles this special case... with a name that makes the distinction clear.
What do you think?

Do you want to write it, @gillisandrew ?

@Matthew-Reidy
Copy link
Contributor

Hello. If @gillisandrew doesn't want to take this I'd like to have a crack at it. Thanks!

@gmlewis
Copy link
Collaborator

gmlewis commented Apr 19, 2024

Thank you, @Matthew-Reidy ! It's yours.

@Matthew-Reidy
Copy link
Contributor

Matthew-Reidy commented Apr 22, 2024

Hey all, I will begin working on this today but I just want to make sure I understand what the problem is here before I start. Basically passing an empty array of repositories as a parameter should return a list of all repos that are a part of an installation but the client discards empty arrays?

@gmlewis
Copy link
Collaborator

gmlewis commented Apr 22, 2024

Hey all, I will begin working on this today but I just want to make sure I understand what the problem is here before I start. Basically passing an empty array of repositories as a parameter should return a list of all repos that are a part of an installation but the client discards empty arrays?

Actually, I'm a bit confused now myself. The original description talks about a POST call, but the example given is a GET call.

@gillisandrew - can you please clarify what your ask is here, and exactly which go-github endpoint(s) you are talking about?

@gillisandrew
Copy link
Author

@gmlewis Hi thanks for following up. The examples given are correct, the -d argument changes the request method to POST as demonstrated in the examples on the man page.

Sorry for the confusion.

@gmlewis
Copy link
Collaborator

gmlewis commented Apr 22, 2024

Oh, cool! TIL. Sorry, @gillisandrew - my bad... both are the POST case.

OK, so @Matthew-Reidy - we need a new endpoint...
The endpoint we are talking about is CreateInstallationToken.

Now the hard part - what should the name be?

Maybe CreateInstallationTokenListRepos ? @gillisandrew - ideas?

Now the easy part is that this new method will have an internal anonymous struct almost identical to InstallationTokenOptions with the one modification:

RepositoryIDs []int64 `json:"repository_ids"`

and this field will not be assigned a value.

@gillisandrew - have I got all this right?

@Matthew-Reidy
Copy link
Contributor

Got it. If there are no objections I'll just move forward with CreateInstallationTokenListRepos as a name for the new endpoint function. If my week goes to plan hopefully I can get a PR in by Friday.

@Matthew-Reidy
Copy link
Contributor

Matthew-Reidy commented Apr 26, 2024

Submitted a PR to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants