Skip to content

[v13] fix!: respect deny rules for access requests#34603

Merged
nklaassen merged 4 commits intobranch/v13from
bot/backport-34438-branch/v13
Nov 16, 2023
Merged

[v13] fix!: respect deny rules for access requests#34603
nklaassen merged 4 commits intobranch/v13from
bot/backport-34438-branch/v13

Conversation

@nklaassen
Copy link
Copy Markdown
Contributor

@nklaassen nklaassen commented Nov 14, 2023

Backport #34438 to branch/v13
Backport #30064 to branch/v13
Backport #30152 to branch/v13

Had to update gravitational/trace to make errors.Is/As work with aggregates

@nklaassen
Copy link
Copy Markdown
Contributor Author

nklaassen commented Nov 15, 2023

tests broken due to gravitational/trace update, I'll probably need to backport #30064 done

Access Request follow their own set of RBAC rules.
Usually, none of the typical create/read/list/delete verbs are required
in any user's roles.
Access is handled via custom rules based on the allow.request, deny.request,
allow.review_requests, and deny.review_requests role fields.

The create/read/list/delete verbs commonly used for other resources are
usually all or nothing (barring `where` expressions), but a more nuanced
set of rules apply to access requests. E.g. users should always be
allowed to see access requests that they created or are allowed to
review, without being allowed to see other access requests in the
cluster.

This seemed mostly logical once you thought about it long enough, but
one detail that has been lacking so far is that explicit deny rules in
the user's roles have no effect at all, even though explicit allow rules
grant god-mode access to create or view any access requests in the
cluster.

Even with the following role, you could still create and view
access requests:

```yaml
kind: role
version: v6
metadata:
  name: example
spec:
  allow:
    request:
      roles: ["*"]
    review_requests:
      roles: ["*"]
  deny:
    rules:
      - resources: ["access_request"]
        verbs: ["create", "read", "list"]
```

This commit makes any explicit deny rules actually take effect.

Fixes gravitational/customer-sensitive-requests#103

changelog: Respect explicit deny rules for Access Requests.
@nklaassen nklaassen force-pushed the bot/backport-34438-branch/v13 branch from f5db40b to f64a79b Compare November 15, 2023 19:05
codingllama and others added 2 commits November 15, 2023 11:20
* chore: Bump gravitational/trace to v1.3.0

* Replace `trace.IsEOF` with `errors.Is`

* Fix IsPermanentEmitError
@nklaassen nklaassen force-pushed the bot/backport-34438-branch/v13 branch from f64a79b to 55fc848 Compare November 15, 2023 19:21
@nklaassen nklaassen added this pull request to the merge queue Nov 16, 2023
Merged via the queue into branch/v13 with commit d43e7dc Nov 16, 2023
@nklaassen nklaassen deleted the bot/backport-34438-branch/v13 branch November 16, 2023 01:28
This was referenced Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants