RFD 186: Optionally require reason for access request

rfd/0186-optionally-require-reason-for-access-request.md

@@ -0,0 +1,215 @@
+---
+authors: Pawel Kopiczko ([email protected])
+state: draft
+---
+
+# RFD 186 - Optionally require reason for access request
+
+
+## Required Approvers
+
+* Engineering: @r0mant && @smallinsky
+* Product: @klizhentas && @xinding33
+
+
+## What
+
+Add the ability to set a policy to make a reason required for an access request
+to be submitted
+([#20164](https://github.com/gravitational/teleport/issues/20164)).
+
+
+## Why
+
+Improves user experience for environments with audit/compliance policies that
+require a reason. Adding the ability to mark a reason as required would allow
+both tsh and the web UI to prompt for a reason without the end user needing to
+remember that it's required and as a result reduce the number of invalid access
+requests that need to be rejected.
+
+
+## Details
+
+
+### Possible implementations
+
+
+#### 1. Add a new value to role.options.request_access
+
+Currently a role can have `request_access` option set to one of the following
+values: `optional`, `always` and `reason` as described in
+https://goteleport.com/docs/admin-guides/access-controls/access-requests/access-request-configuration/#how-clients-request-access.
+
+The idea is to support another value `reason-required`.
+
+> [!NOTE]
+> role.options.request_access is a global configuration
+
+It has to be noted that role.options.request_access has a global scope. I.e. if
+it's set on any role all access requests to all roles and resources are
+affected.
+
+Example:
+
+For two roles `rr-role` and `dba-req-role` as defined below:
+
+```yaml
+kind: role
+version: v7
+metadata:
+  name: rr-role
+spec:
+  options:
+    request_access: reason-required
+```
+
+```yaml
+kind: role
+version: v7
+metadata:
+  name: dba-req-role
+spec:
+  allow:
+    request:
+      roles:
+        - 'dba'
+```
+
+A user with both roles `rr-role` and `dba-req-role` assigned will be affected
+by request_access set in role `rr-role` while requesting access to role `dba`
+allowed by role `dba-req-role`.
+
+Because request_access option is global a priority has to be establish when
+more than one role has the option specified:
+
+- reason
+- always
+- reason-required
+- optional (default)
+
+Problems:
+
+- It may be confusing/undesired that roles with request_access option set to
+  "reason-required" affects other roles.
+
+
+#### 2. Add a new value to role.options.request_access and change the option to be scoped
+
+> [!NOTE]
+> This is a breaking change.
+
+The solution would be like in 1. but with changed semantics of request_access
+option. It would become role-scoped instead of global.
+
+Example:
+
+For two roles `dba-requester` and `ssh-requester` as defined below:
+
+```yaml
+kind: role
+version: v7
+metadata:
+  name: ssh-requester
+spec:
+  allow:
+    request:
+      roles:
+        - 'ssh'
+  options:
+    request_access: reason-required
+```
+
+```yaml
+kind: role
+version: v7
+metadata:
+  name: dba-requester
+spec:
+  allow:
+    request:
+      roles:
+        - 'dba'
+  options:
+    request_access: optional # default, doesn't need to be set explicitly
+```
+
+A user with `ssh-requester` and `dba-requester` roles assigned would have to
+provide a reason while creating an access request for role `ssh` but wouldn't
+have to provide reason while requesting access to `dba`.
+
+The example above is clear and simple, but there are problems with the
+implementation:
+
+- **This is a breaking change.** It may come as a surprise to the existing
+  users that only _some_ instead of _all_ roles are automatically requested on
+  login when request_access is set to "always" or "reason".
+- What happens when there are 2 or more roles allowing requesting the same
+  role/resource but they have different request_access set? Should the solution
+  fall back to priorities, like in 1.?
+
+
+#### 3. Add a new role.spec.allow.request.reason_required setting
+
+This is pretty self-explanatory. role.spec.allow.reason_required could be set
+to true/false and being false by default.
+
+Example:
+
+```yaml
+kind: role
+version: v7
+metadata:
+  name: dba-requester
+spec:
+  allow:
+    request:
+      require_reason: true
+      roles:
+        - 'dba'
+```
+
+
+The problems with the approach:
+
+- It isn't clear what should happen when when there is a role with
+  `options.request_access: always` and a role with
+  `allow.request.reason_required: true`. Should the reason be requested
+  regardless during login?
+- Future - as an extension of the problem above: there were ideas of having more
+  advanced requirements when it comes to providing a reason, e.g. using regular
+  expressions. In case of `options.request_access: reason` - how to satisfy all
+  of them? 
+- Minor: it will be possible to set `role.spec.deny.request.require_reason` (the same
+  type is being reused in the code) but it will do nothing.
+
+
+### Limitations
+
+- In the web UI, currently `role.options.request_access` option is evaluated
+  only during login, so the user has to re-login to have a correct UX if there
+  are changes to their assigned roles.
+- Depending on the chosen solution the implementation may be quite involved for
+  the value it provides.
+
+
+### Decision
+
+TODO pick a solution and cover the edge-cases.
+
+
+### Test Plan
+
+The Request Access section of the test plan needs to be extended with these
+items:
+
+- TBD depending on the chosen implementation.
+
+
+### References
+
+- Old RFD:
+  https://github.com/gravitational/teleport/blob/master/rfd/0003-extended-approval-workflows.md
+- request_access documentation:
+  https://goteleport.com/docs/admin-guides/access-controls/access-requests/access-request-configuration/#how-clients-request-access
+- Private Slack thread:
+  https://gravitational.slack.com/archives/C05M9TC5WKD/p1729799236724089