RFR: 1903: Verify User's group membership when processing backport command [v2]

[Zhao Song]

On Tue, 9 May 2023 21:54:02 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> Zhao Song has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update bots/pr/src/main/java/org/openjdk/skara/bots/pr/BackportCommand.java
>>   
>>   Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/BackportCommand.java line 393:
> 
>> 391:     }
>> 392: 
>> 393:     private boolean verifyGroupMembership(HostedRepository targetRepo, HostUser user, PrintWriter reply) {
> 
> The name of this method is misleading as we aren't actually checking group membership, in this patch we are checking write access, which may or may not be indicative of group membership. What actually matters is that the right access is available though.

Will change the name to `checkWriteAccess`. Thanks!

> bots/pr/src/main/java/org/openjdk/skara/bots/pr/BackportCommand.java line 398:
> 
>> 396:         if (!targetRepo.forge().name().equals("GitHub") && !targetRepo.forge().name().equals("Test")) {
>> 397:             hasGroupMembership = targetRepo.canPush(user);
>> 398:         }
> 
> This is too much of a hack for me. There needs to be a method on the repository that each relevant implementation of the `HostedRepository` interface has a valid implementation for that answers the question we need to ask. I think that question is something like "is the user allowed to open pull requests against this repo"? For `GitHubRepository` that is probably always true, at least in our context. For `TestHostedRepository` I would assume it's always true, unless you wanted to make it false for testing purposes. In that case you could make it possible to toggle from a test. For `GitLabRepository` the method can probably delegate to `canPush`, but I would like to have that verified.

It's quite a good idea. Will do it. Thanks!

-------------

PR Review Comment: https://git.openjdk.org/skara/pull/1516#discussion_r1189180192
PR Review Comment: https://git.openjdk.org/skara/pull/1516#discussion_r1189180287