Support more fields in the role editor #51458
Conversation
bl-nero
changed the title
bl-nero
force-pushed
the
bl-nero/role-editor-more-fields
branch
from
e946468 to
4ebf9db
Compare
bl-nero
added
the
no-changelog
| <Text typography="body3" mb={1}> | ||
| Labels | ||
| </Text> | ||
| <FieldSelectCreatable |
There was a problem hiding this comment.
the inputs placeholder says Select..., are we supposed to have a list of users to select? or maybe replace with Start typing a user and press enter?
There was a problem hiding this comment.
Good idea; added in other places, too.
| /> | ||
| </Box> | ||
| <LabelsInput | ||
| legend="Labels" |
There was a problem hiding this comment.
should we also add a tooltip explaining labels as well?
| /> | ||
| <LabelsInput | ||
| legend="Database Service Labels" | ||
| tooltipContent="The database service labels have no influence on which databases' access is controlled by this role. Instead, they control which database services are discoverable while enrolling a new database." |
There was a problem hiding this comment.
Instead, they control which database services are discoverable while enrolling a new database.
tbh, i didn't understand what this means 😢, can we simplify it? The ai doc says:
There was a problem hiding this comment.
@kimlisa Take a look at this discussion. @marcoandredinis wrote this part, so perhaps you two could help me rephrasing this explanation to be easier to understand?
There was a problem hiding this comment.
I would phrase it like this
The database service labels controls which Database Services (Teleport Agents) are visible to the user. Access to Databases is controlled by the Databased Labels field.
We can add more details if this is not clear.
The database service labels controls which Database Services (Teleport Agents) are visible to the user, required when adding Databases in the Enroll New Resource wizard. Access to Databases is controlled by the Databased Labels field.
There was a problem hiding this comment.
i think marco's first quote box is good enough, we can perhaps add a link to Database Service that explains what it is too
or this is good too:
The database service labels have no influence on which databases' access is controlled by this role. Instead, they control which Database Services (Teleport Agents) are visible to the user."
| @@ -112,6 +115,26 @@ const AccessRule = memo(function AccessRule({ | |||
| value={verbs} | |||
| onChange={v => setRule({ ...value, verbs: v })} | |||
| rule={precomputed(validation.fields.verbs)} | |||
| /> | |||
| <FieldInput | |||
| label="Condition" | |||
There was a problem hiding this comment.
personally i think it's okay to label it this way, but i'd also double check with product team because we try to teach users consistent wording
There was a problem hiding this comment.
After consulting UX/Product, we decided to rename it to Filter.
| Additional condition, expressed using the{' '} | ||
| <Text | ||
| as="a" | ||
| href="https://goteleport.com/docs/reference/predicate-language/" | ||
| target="_blank" | ||
| color={theme.colors.interactive.solid.accent.default} | ||
| > | ||
| Teleport predicate language | ||
| </Text> |
There was a problem hiding this comment.
what about something like: Optional condition further limiting the scope for this rule expressed using the ....
bl-nero
changed the title
bl-nero
force-pushed
the
bl-nero/role-editor-more-fields
branch
from
4ebf9db to
cdcd860
Compare
|
Sorry for the force-push, I had to reconcile a messed up merge of the PR that I was depending on here. |
This PR adds the following fields:
whereclause in access rules. I took liberty to call it "Condition" in the UI, so please voice your opinion on whether this is a good choice. My line of reasoning is that without understanding the underlying YAML model, the name "Where" would be confusing.The intent behind this PR was to fully support the
accessrole in the rich UI, but it turned out that one more field was added to it, so we still miss the target. However, I added warnings to the Go codebase now, so hopefully, this will make it easier to prevent these situations in future.Requires #51457