Skip to content

Add integration access rule to web user context#24256

Merged
kimlisa merged 5 commits intomasterfrom
lisa/discover/add-integration-to-user-context
Apr 11, 2023
Merged

Add integration access rule to web user context#24256
kimlisa merged 5 commits intomasterfrom
lisa/discover/add-integration-to-user-context

Conversation

@kimlisa
Copy link
Copy Markdown
Contributor

@kimlisa kimlisa commented Apr 6, 2023

in response to #24133
part of #22129

Add the new integrations rule to the web ui user context

@kimlisa kimlisa enabled auto-merge April 6, 2023 23:46
@kimlisa kimlisa added this pull request to the merge queue Apr 7, 2023
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 7, 2023
Comment thread lib/web/ui/usercontext.go
// Plugins defines whether the user has access to manage hosted plugin instances
Plugins access `json:"plugins"`
// Integrations defines whether the user has access to manage integrations.
Integrations access `json:"integrations"`
Copy link
Copy Markdown
Contributor Author

@kimlisa kimlisa Apr 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marcoandredinis i just realized that the frontend will need to check for both integration.create and integration.use before starting the aws integration b/c at the list rds dbs screen we will need to use the integration?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes 👍
We'll need the use verb in order to call Integration's APIs

Copy link
Copy Markdown
Collaborator

@zmb3 zmb3 Apr 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need a new verb for this?

I'm concerned that we just add random verbs like this that are only used for one particular feature and then never document them, and after doing this for years we have a confusing RBAC system that no one can understand.

For example, I can totally see a support ticket coming in that says "my role has the use verb for desktops but I can't connect to the desktop, why?"

Is there a way to use the existing RBAC system that we have today rather than extend it just for thi feature?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, we considered other options but ended up with the use verb
Do you think one of the following would be better?

  • another role option: integration_labels
  • re-use another "standard" verb to check when calling a 3rd party API: integration.create?
  • change the verb to execute and hopefully it can become a standard verb from now on to indicate that the role allows execution of some kind in the resource (ssh into a node, kube exec in a pod, ...)

cc @r0mant

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have other existing verbs we can reuse here tbh. Other verbs we have are either for CRUD actions, or were also introduced specifically for individual features. For example, we have VerbRotate, VerbCreateEnrolleToken, VerbEnroll: https://github.com/gravitational/teleport/blob/master/api/types/constants.go.

Compared to these, VerbUse is actually less "random" concept IMO, for example Kubernetes uses it (no pun intended) in its RBAC too.

Copy link
Copy Markdown
Contributor

@marcoandredinis marcoandredinis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
We can add the use verb later now or still in this PR

@kimlisa kimlisa enabled auto-merge April 11, 2023 14:43
@kimlisa kimlisa added this pull request to the merge queue Apr 11, 2023
Merged via the queue into master with commit a5e3079 Apr 11, 2023
@kimlisa kimlisa deleted the lisa/discover/add-integration-to-user-context branch April 11, 2023 16:20
kimlisa added a commit that referenced this pull request May 3, 2023
* Add integration access rule to web user context (#24256)

* Add the new access verb `use` to web user context (#24463)

* BE: Add field for verb use for user ACL

* FE: Add the new use access verb

* Only define use verb for relevant resources
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants