[automate-2710] Remove system:* perms, modify infra:* perms

[blakestier]

🔩 Description: What code changed, and why?

System actions are intended for internal use and are needed for Automate to work correctly (i.e. must be able to check the license status, the IAM version, etc. for the UI to render things correctly). They are not related to permissions a user might perform on Automate resources (i.e. create a user, delete a missing node). To keep system policies hidden from end users, we load them directly into OPA cache, keeping them out of policy database.

Since system actions are not intended for end users to use in their policies, actions like system:*:list, system:*:get are not needed on the viewer role

The only thing the editor gains with the system:* is the ability to apply a license, but we want that to be admin-only anyway.

We need to adjust our editor and viewer roles to be more explicit about what is allowed under infra because chef servers is getting added.

SCOPE CREEP:

In the making of this PR, we noticed 1) Roles were only accidentally getting access to telemetry due to the system:* action (the existing telemetry:* action was retired w/ v2 and 2) Ppl outside of Admin had the capacity to request/apply license and see the gateway endpoint due to system:*. . . we've tightened those perms here.

Ideally, we have a follow-up to move telemetry and license out of the system namespace, but this gives us the desired functionality.

👍 Definition of Done

• system prefixed actions are removed from Chef-managed roles
• Update documentation, specifically..
• iam-v2-overview/#role-based-access-control: note that editors cant apply a license
• iam-v2-api-reference/#default-chef-managed-roles: update the table
• in the editor role, change infra:* to infra:nodes:*, infra:nodeManagers:*
• in the viewer role, change infra::get, infra::list to infra:nodes:get, infra:nodes:list, infra:nodeManagers:get, infra:nodeManagers:list
• change the infra legacy policy to migrate as infra:nodes:* and infra:nodeManagers:* instead of infra:*
• only admin have access to requesting/applying licenses
• all users have access to the telemetry endpoint

👟 How to Build and Test the Change

1. start_all_services
2. force-upgrade: rebuild components/authz-service/ && rebuild components/automate-cli && rebuild components/automate-deployment/ && rebuild components/automate-gateway && rebuild components/teams-service/
3. export TOK=chef-automate iam token create adm --admin
4. curl updated legacy policy:

curl -kH "api-token: $TOK"  https://localhost/apis/iam/v2/policies/infrastructure-automation-access-legacy | jq

{
  "policy": {
    "name": "[Legacy] Infrastructure Automation Access",
    "id": "infrastructure-automation-access-legacy",
    "type": "CUSTOM",
    "members": [
      "user:*"
    ],
    "statements": [
      {
        "effect": "ALLOW",
        "actions": [
          "infra:nodes:*",
          "infra:nodeManagers:*"
        ],
        "role": "",
        "resources": [
          "*"
        ],
        "projects": [
          "*"
        ]
      }
    ],
    "projects": []
  }
}

the 'infrastructure-automation-access-legacy policy no longer has infra:* statement actions (but infra:nodes:* and infra:nodeManagers:* instead).

Builds:
Verify Private

Verify

[blakestier]

everyone should have this access, and we were inadvertently denying it to all users, and providing it to certain roles by accident due to the system:* namespace.

[blakestier]

expected functionality for v2 has changed, so the a2-iam-no-legacy-integration won't pass on both the pre and post upgrade. Smoke tests ensure the pre-upgrade is running.

[msorens]

Looks good; just a couple things I noticed.

[msorens]

I think you missed applications for viewer (you have it correctly for editor below):

automate/components/authz-service/storage/postgres/datamigration/sql/07_update_v2_roles.up.sql

Line 5 in 7392f96

actions = actions || '{applications:*:list, applications:*:get}'

[msorens]

If the docs are valid, project owner is supposed to be a superset of editor, so it also needs applications:*.

[bcmdarroch]

Even though it's not necessary with this migration, could we change the migration code to reflect the correct migration for reference?

[blakestier]

I'd prefer not to just change the migration for the policy in the migration code and not also change the roles, and keep this migration.

If it's really clutch that try to preserve the state of the world in the force-upgrade branch, I'd rather modify the migration code for policies AND roles and associated tests, and the datamigrations, and not add an additional SQL migration. Or I can pull this work out and point it to master, and then we'll need to be careful to fix the things in force-upgrade.

FWIW, this is why I didn't just change the migration like we'd discussed (these mostly assume that if I change the policy in the migration code, I should also change the roles (if the reason is to preserve the state of the world))

1- since this is a post migration migration (not just a data migration), then it's kind of weird that we're changing both-- if I'm strictly looking at the code in the future, I'm like "why do we have this migration if all the migration code already handles this?"

2- The other thing is that the datamigrations will overwrite the migration code-- so we'll create the roles in the v2 upgrade, then overwrite them in the datamigrations, then overwrite them again in the post v2 migrations. Not a huge deal, but feels off.

3- this change isn't really immediately related to the force upgrade-- I've put it here only bc we'd have to do a few modifications to force-upgrade in order to make it happy with this change.

I agree that it's annoying that we don't have a single source of truth, but I think that this will only be a source of truth for a moment in time, and we'll need to reference the sql migrations after that.

What do you think?

[tylercloke]

If there's not a functional reason to update the migration code, I'm a :bigfan: of only ever touching that code again if necessary. It's a lot of high risk code we don't wanna maintain long-term.

[bcmdarroch]

sounds good

[bcmdarroch]

👍 had one quick ask but looks good to go!

[susanev]

there is a do not merge label on this pr because its currently pointed at master
it needs to merge into the force-upgrade branch