Add the ability to add/remove console users from a Google Group

[jianglai]

If a console user group is defined in the config file, we will add/remove the user to/from the group during user creation/deletion, instead of granting/removing the necessary IAP roles to/from the user.

This makes it easy when IAM roles are managed externally (e. g. under latchkey) where individual changes made to IAM will be rolled back, if not also made to latchkey. This way we can simply grant the role to the group via an one-time config change to latchkey, and rely on a user's membership in the group to determine if IAP would all or deny the user's access.

Also forbid mocking of Response in tests. We should use the fake implementation instead.

TESTED=tested both individual and group IAP behavior on crash. Verified that email addresses are added/removed to/from the group.

This change is 

[jianglai]

Reviewed 20 of 20 files at r1, 4 of 4 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Github-advanced-security[bot] and @ptkach)

---

core/src/test/java/google/registry/tools/server/UpdateUserGroupActionTest.java line 35 at r1 (raw file):

Previously, github-advanced-security[bot] wrote…

Unused classes and interfaces

Unused class: UpdateUserGroupActionTest is not referenced within this codebase. If not used as an external API it should be removed.

Show more details

False positive. This is a test class.

[ptkach]

Reviewed 5 of 20 files at r1, 2 of 4 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Github-advanced-security[bot] and @jianglai)

---

core/src/main/java/google/registry/tools/CreateUserCommand.java line 56 at r4 (raw file):

  protected String execute() throws Exception {
    String ret = super.execute();
    if (maybeGroupEmailAddress.isPresent()) {

An assumption is made here that when group email is present we always add to the group, this locks us into this assumption. I'm guessing this might be confusing for someone not familiar with what we're trying to achieve and why, like when group is present or when it's missing, etc.
Would it make more sense to expand the command and tools with param that would accept the groupname to add to? I'm thinking this might make things more straighforward and clear

[jianglai]

Reviewed 6 of 7 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Github-advanced-security[bot] and @ptkach)

---

core/src/main/java/google/registry/tools/CreateUserCommand.java line 56 at r4 (raw file):

Previously, ptkach (Pavlo Tkach) wrote…

An assumption is made here that when group email is present we always add to the group, this locks us into this assumption. I'm guessing this might be confusing for someone not familiar with what we're trying to achieve and why, like when group is present or when it's missing, etc.
Would it make more sense to expand the command and tools with param that would accept the groupname to add to? I'm thinking this might make things more straighforward and clear

The behavior is well documented in default-config.yaml. I don't think we want to make it possible to specify an arbitrary group email address on the command line, as that should be part of the configuration that is under version control.

I do think it make sense to send the group as an argument to the backend so the backend doesn't rely on hard-coded group email addresses.

[jianglai]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Github-advanced-security[bot] and @ptkach)

[jianglai]

Gentle ping.

[ptkach]

Looking at the IAP ticket that's been bouncing around I'm not 100% that this approach will get to fly. Thus I'm waiting for the to approve the change first. Otherwise we will go with allAuthorized approach and that will make this PR obsolete

Reviewable status: 19 of 20 files reviewed, 2 unresolved discussions (waiting on @Github-advanced-security[bot] and @jianglai)

[jianglai]

Looking at the IAP ticket that's been bouncing around I'm not 100% that this approach will get to fly. Thus I'm waiting for the to approve the change first. Otherwise we will go with allAuthorized approach and that will make this PR obsolete

Reviewable status: 19 of 20 files reviewed, 2 unresolved discussions (waiting on @Github-advanced-security[bot] and @jianglai)

I think this PR is a net positive regardless of what latchkey does. It provides an alternative to granting the IAP role to users individually, without taking any functionalities away.

We should also consider that currently latchkey has not reverted the role for the groups that I manually added for a couple of days. There is no telling if it will eventually revert the change. But if it does (which means we have to go through latch key to grant any IAM related roles), it would be much easier to get approval for selected Google groups under a domain that we control, than for allAuthorized.

[ptkach]

Reviewed 6 of 20 files at r1, 6 of 7 files at r5, 1 of 1 files at r6, 1 of 1 files at r7, 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Github-advanced-security[bot])