Improve help text for admin permissions (FGAP) fields - #51538
Conversation
There was a problem hiding this comment.
Pull request overview
Improves Admin UI guidance for fine-grained admin permissions and prevents group evaluation from crashing.
Changes:
- Expands permission and policy help text.
- Adds missing scope and policy-name guidance.
- Handles uninitialized group selections safely.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
GroupSelect.tsx |
Defaults missing group values to an empty array. |
NewPermissionPolicyDialog.tsx |
Adds policy-name help text. |
messages_en.properties |
Adds and expands permission help messages. |
Suppressed comments (1)
js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties:3947
clientScopesHelpis shared with the normal resource editor viaResourceDetails.tsx:232, so there it incorrectly tells users that the selected scopes belong to a permission. Use wording that is valid for both resources and FGAP permissions, or select a context-specific key inScopePicker.
clientScopesHelp=Scopes represent the operations that can be performed on a resource type (e.g. 'view', 'manage', or resource-specific like 'manage-members' for groups). Each scope is independent, so granting 'manage' does not automatically grant 'view'. Select the scopes this permission should enforce.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (4)
js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties:3948
- Policy and permission names share the same
PolicyStoreuniqueness check (PolicyService.java:155-163, also used byPermissionService), so reusing an existing permission name returns a conflict. This help text should identify both kinds of conflicting names.
policyNameHelpText=A unique name for the policy. The name must not conflict with any existing policy name.
js/apps/admin-ui/src/permissions-configuration/resource-types/GroupSelect.tsx:43
- Please add a Playwright regression case that opens Evaluation, selects a user, and then selects the Groups resource type. The existing Evaluation tests cover only Clients (
test/permissions/main.spec.ts:150-174), so they would not catch this crash when the group field mounts unregistered.
const values: string[] = getValues(name!) || [];
js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties:3947
- This help text is shared with the normal authorization resource editor (
ResourceDetails.tsx:232), where no permission is being edited, so “this permission should enforce” is incorrect there. Use a separate FGAP key or wording that is accurate in both contexts.
clientScopesHelp=Scopes represent the operations that can be performed on a resource type (e.g. 'view', 'manage', or resource-specific like 'manage-members' for groups). Each scope is independent, so granting 'manage' does not automatically grant 'view'. Select the scopes this permission should enforce.
js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties:3859
- Permission and policy names share the same
PolicyStoreuniqueness check (PolicyService.java:155-163, also used byPermissionService), so reusing an existing policy name returns a conflict. This help text should identify both kinds of conflicting names.
This issue also appears on line 3948 of the same file.
permissionNameHelpText=A unique name for the permission. The name must not conflict with any existing permission name.
| createPermissionOfType=This permission will be applied to the {{resourceType}} | ||
| permissionUsersHelpText=Specifies which user(s) are allowed by this permission. | ||
| permissionNameHelpText=The name of the permission. This name is used to identify the permission in the admin console. | ||
| permissionNameHelpText=A unique name for the permission. The name must not conflict with any existing permission name. |
There was a problem hiding this comment.
It actually must not conflict with any existing permission or policy name - both are stored in the same table (RESOURCE_SERVER_POLICY), so the name must be unique as per the constraint on (NAME, RESOURCE_SERVER_ID).
There was a problem hiding this comment.
Looks like Copilot beat me to it while I was typing :)
| createPermissionPolicy=Create policy | ||
| enforceAccessTo=Enforce access to | ||
| enforceAccessToHelpText=Specifies the resource that the permission is applied to. | ||
| enforceAccessToHelpText=Defines whether this permission applies to all resources of the selected type or only to specific ones. Resource-specific permissions take precedence over all-resource permissions during evaluation. |
There was a problem hiding this comment.
nit pick, but take precedence is slightly imprecise. As per the documentation, when resource-specific permissions exist, the all-resource permission is "NOT taken into account" at all. It's completely skipped, not just outranked. "Take precedence" could be read as "both are evaluated but specific wins" when the actual behavior is that the all-resource permission is entirely excluded.
Closes keycloak#51537 Signed-off-by: Martin Bartoš <mabartos@redhat.com>
Closes keycloak#51536 Signed-off-by: Martin Bartoš <mabartos@redhat.com>
clientScopesHelppolicyNameHelpText@keycloak/core-iam Could you please check it? I think the permissions/policies might be more clear from the Admin UI perspective without touching the docs + fixing some minor issues in the Admin UI. Let me know WDYT. Thanks!