Add automatic role access requests

[atburke]

This change adds the option to automatically make a role access request during tsh ssh instead of the current automatic resource access request. See core goals for details on expected functionality.

$ tsh ssh --request-mode role [email protected]
Error: access denied to user connecting to node.example.com:0

You do not currently have access to "user@node", attempting to request access.

Choose role to request [role-1, role-2, role-3]: # user enters role name
Enter request reason: # user enters request reason
# the rest is the same as for resource requests

Changelog: Added automatic role access requests

[fspmarshall]

This is going to need some tweaking to get the access-control model correct. roles and search_as_roles are more different than they initially appear because search_as_roles implicitly grants read access to all resources in the list, but roles does not. We have to ensure that this API does not provide an opening for a user to test for the existence of resources for which they do not have read access. Similarly, the auth server is unable to make decisions about resources in leaf clusters, meaning that it will likely actually be common for us to be given a resource ID list where the answer to the question "what roles can I request to get access to this resource?" is undecidable, either for technical or security reasons. Given the above, I think probably this API should just fallback to returning the entire list of all roles the user has the ability to request if either A) one or more IDs is not a resource that the user already has read access to, or B) one or more IDs is for a resource in a different cluster.

Also, I think @nklaassen should be a required reviewer for this work given his expertise in this kind of thing.

[atburke]

Friendly ping @nklaassen

[nklaassen]

Is there a GH issue or anything motivating this you could link to?

If there's not a design doc anywhere, could you include in the PR description a high level description of the usecase, the design, and an example of what this looks like on the CLI when you use it?

[nklaassen]

roles and search_as_roles are more different than they initially appear because search_as_roles implicitly grants read access to all resources in the list, but roles does not. We have to ensure that this API does not provide an opening for a user to test for the existence of resources for which they do not have read access

This is a great point and I'm not coming up with a good solve for it off the top of my head. The test for VerbRead in the current PR is not sufficient because we normally don't allow read access unless node_labels or node_labels_expression matches as well

[atburke]

Friendly ping @nklaassen @fspmarshall

[nklaassen]

I'd still like for the PR description to include example of what it looks like to use this in the CLI

[fspmarshall]

I think the idea of just needing an overlapping search_as_roles declaration in order to view target resources is a good one. Eventually we may want to give the UX some additional polish, possibly some kind of flag that could be set to switch the roles field to imply implicit reads so that users don't need to maintain two sources of truth for requestable roles. I think the overlapping search_as_roles idea is good enough for now though, as it lets us do the right thing from an access control perspective without adding new knobs and/or doing anything too unexpected.

Some additional test coverage in lib/auth and/or lib/services would be good (tsh adds a lot of client-side behavior, so I think its good practice to verify security-related behaviors without it in the mix). As Nic mentioned, at least one or two tests should simulate a user probing with resource IDs they aren't allowed to know about.

Overall though, I think the design is in a good place!

[nklaassen]

Sorry to go back and forth so many times on this, it's close! I think we've got the backend right now, I just have a couple nits, and then some comments on the tsh UX which I think can be improved a bit

[nklaassen]

Thanks for addressing all of my comments, looks good!

[nklaassen]

I would handle setAccessRequestReason from within promptUserForAccessRequestDetails, logically it's part of that

[atburke]

@zmb3 @r0mant Can I get a flaky test skip from one of y'all? I think TestSSHAccessRequest is too slow to make it through.

[zmb3]

/excludeflake *

[public-teleport-github-review-bot]

@atburke See the table below for backport results.

Branch	Result
branch/v15	Failed