Checking for user permissions will fail if user is a member of the nested team

[yafanasiev]

Hi! First of all, thanks for a wonderful action. We do use it a lot.

We hit an issue where users who have write access to the repository can't trigger slash commands when permission property is set to write. After investigation, I found out that the issue occurs if user has permissions on repository through the nested team (so the team main is assigned write permissions on the repo, it has sub-team sub, and user is a member of team sub). I tried to reproduce the calls that actions does locally (specifically

slash-command-dispatch/src/github-helper.ts

Line 57 in 12905bc

async getActorPermission(repo: Repository, actor: string): Promise<string> {

and it appers that for those users Github's API returns empty reponse. After reaching out to Github support, they acknowledge this as a intendent behaviour for now, to quote them

the collaborators relation only returns first-level users (users that are directly added to repository) or members of the team that is added to the repository, but not nested teams

and this is something that could change in the future, but not right now.
I do understand that this is something relatively specific to our use case, but I am adding this in case someone hits the same problem and does not waste as much time as we did. I also checked REST API version of this request , and it returns the correct list of collaborators. This could server as a substitute for current implementation, but would require getting all of the collaborators and then filtering by current actor name, which is not quite neat as the GraphQL version. So it is up to mainteners to make the call, but at least this could be added to README somewhere to save others some time.

[peter-evans]

Hi @yafanasiev

Thank you for reporting this and for your proactive effort to understand the problem.

The original version of the getActorPermission function looked like this:

slash-command-dispatch/src/github-helper.ts

Lines 45 to 53 in 0495b44

	async getActorPermission(repo: Repository, actor: string): Promise<string> {
	const {
	data: {permission}
	} = await this.octokit.repos.getCollaboratorPermissionLevel({
	...repo,
	username: actor
	})
	return permission
	}

I modified it to use the GraphQL API in this PR so that the action could support the triage and maintain permission levels, which are not accessible through the REST API.

The original version used this REST API. Perhaps this API works for users in a nested team.

I don't know if there is a single GitHub API that works for fetching the permission of users in a nested team AND returns the triage and maintain permission levels. I'll have to spend some more time to research it. If you find out any information that could help, please let me know.

[yafanasiev]

So I did a little bit of digging and it seems like there is no clear way to resolve this. The REST API that was used before still does not include new permissions, despite a lot of requests on community forums. I played a bit with List repository collaborators API and it looks like it does contain the necessary information:

...
        "type": "User",
        "site_admin": false,
        "permissions": {
            "admin": false,
            "maintain": false,
            "push": false,
            "triage": true,
            "pull": true
        }
...

albeit the naming is a bit off - read is pull and write is push. However that would mean looping through entire list of repository collaborators looking for current user, which could be potentially slow. Maybe this could be implemented as switchable behaviour? Although I see how this overcomplicates things. Anyway, I can create PR either for this or for a notice in docs, whichever you think would be best @peter-evans

[peter-evans]

Thank you for looking into it further. It's disappointing that there doesn't seem to be a good solution for this at the moment.

I would like to avoid having switchable behaviour for this if possible. Also, I'm not keen on using the List repository collaborators API. I think it will be too much of a performance hit for large orgs.

What about having a fallback to the Get repository permissions for a user API I was using originally? The limitation would be that you can't use maintain and triage roles with nested team users. For example, I would guess that for a nested team user if their role is actually maintain the REST API returns write, and downgrades their role.

So what I'm thinking for the getActorPermission function is to try using the GraphQL API to fetch the role first. If there is no result then try the REST API. What do you think?

[yafanasiev]

That could be a workaround, but I think that would complicate the logic too much. If we could detect the nested team and warn user before evaluating permissions then it would be viable, but as of now this looks a bit too complex. I suggest to stay aligned with Github's API on that and add a warning in the docs for nested team users. Hopefully Github will update their API in the near future and problem will kinda resolve itself. I will raise the issue with our team internally to move away from nested teams at all as this functionality looks unfinished from Github's side.

[peter-evans]

It's not ideal, sure, but I think it would be acceptable in this case if it's the only realistic way nested team users can use the action. I wouldn't be too hopeful that GitHub will fix this soon.

In your case it sounds like you are able to transition to non-nested teams, so that's good news. I'll leave this issue open and pinned for others to find, and if we have more caught by this problem then I'll consider implementing something.

Thanks again for raising this issue.

[yafanasiev]

@peter-evans we moved away from nested teams eventually. It adds some overhead, but in the end is more reliable for any kind of automation. I added a PR to add a note in the docs about this.

[rravi-sift]

We started seeing this issue for config that don't have permission check as well.

Config

##[debug]  config: '[\n' +
##[debug]    '  {\n' +
##[debug]    '    "command": "ping",\n' +
##[debug]    '    "issue_type": "pull-request",\n' +
##[debug]    '    "permission": "none",\n' +
##[debug]    '    "repository": "Owner/github-actions-repo"\n' +
##[debug]    '  },\n' +

Log

##[debug]  name: 'GraphqlError',
##[debug]  request: {
##[debug]    query: 'query CollaboratorPermission($owner: String!, $repo: String!, $collaborator: String) {\n' +
##[debug]      '      repository(owner:$owner, name:$repo) {\n' +
##[debug]      '        collaborators(query: $collaborator) {\n' +
##[debug]      '          edges {\n' +
##[debug]      '            permission\n' +
##[debug]      '          }\n' +
##[debug]      '        }\n' +
##[debug]      '      }\n' +
##[debug]      '    }',
##[debug]    variables: { owner: 'some-repo-owner', repo: 'code', collaborator: 'someone' }
##[debug]  }
##[debug]}

[dcermak]

I have unfortunately hit exactly this issue right now again. Since the situation hasn't improved, could we maybe implement a workaround (one that doesn't entail moving away from subteams)?