Repository navigation
Implement selecting specific organizations in FGAP permission #51717
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
69 commits
Select commit
Hold shift + click to select a range
0407296
Safely Adapted organization modal for use with fgap
theohh0 66a947f
Create OrganizationSelect component
theohh0 85f1d26
Add organizations to resource types
theohh0 182f654
Added en translation keys
theohh0 de3b91e
added orgs to permission eval
theohh0 99af335
Show error when no orgs selected
theohh0 fe82415
disable add/join button if no orgs selected
theohh0 b074544
display error message when no organizations exist
theohh0 0ef99c1
filter out organization resource type if disabled
theohh0 ce91a27
Catch not found organizations
theohh0 63fb8da
pass isRadio through to organization modal
theohh0 0292c3a
Clean up organization modal titles code
theohh0 95b9c78
casing and placement
theohh0 52977d3
Don't show empty orgs error when orgs loading.
theohh0 3db6683
Disable existing orgs selected rather than removing them from modal
theohh0 3e9519a
Select all shouldnt select disabled rows in `KeycloakDataTable`
theohh0 f322af7
fix adminclient org typing and catch nulls
theohh0 b58ee47
use translation keys for OrganizationModal.tsx titles
theohh0 45c827c
Missing message keys
theohh0 c53c51d
Calculate orgsEnabled outside of memo
theohh0 dd7d0e8
Don't include subrows in select all
theohh0 fc792ee
remove unnecessary properties from SubRow
theohh0 aa19abf
disable select all if nothing selectable
theohh0 f15306a
don't need to pass existingOrgs length into loader params now that th…
theohh0 f5be7ea
added select/delete org permission test
theohh0 ed4be68
enable organizations in test realm
theohh0 0fded8e
reusable selectedRows var
theohh0 313591b
hopefully more concrete org row selector
theohh0 8bd6f1c
create policy with different name
theohh0 ef5bf38
just check for presence of data property - not truthiness
8000
theohh0 ee7f6ec
clean up perm tests once complete
theohh0 208e112
Safely Adapted organization modal for use with fgap
theohh0 8dd9b80
Create OrganizationSelect component
theohh0 c542dda
Add organizations to resource types
theohh0 f081ae2
Added en translation keys
theohh0 92a247f
added orgs to permission eval
theohh0 2713237
Show error when no orgs selected
theohh0 dd3002a
disable add/join button if no orgs selected
theohh0 9b9d01d
display error message when no organizations exist
theohh0 4036982
filter out organization resource type if disabled
theohh0 bfddda6
Catch not found organizations
theohh0 b2cbb3f
pass isRadio through to organization modal
theohh0 58ea82d
Clean up organization modal titles code
theohh0 8a4e925
casing and placement
theohh0 2903158
Don't show empty orgs error when orgs loading.
theohh0 b4452b7
Disable existing orgs selected rather than removing them from modal
theohh0 cfecd20
Select all shouldnt select disabled rows in `KeycloakDataTable`
theohh0 8109afd
fix adminclient org typing and catch nulls
theohh0 60e6aac
use translation keys for OrganizationModal.tsx titles
theohh0 0dc5936
Missing message keys
theohh0 ab68248
Calculate orgsEnabled outside of memo
theohh0 176c4eb
Don't include subrows in select all
theohh0 60bf891
remove unnecessary properties from SubRow
theohh0 5f4b697
disable select all if nothing selectable
theohh0 b90f8e3
don't need to pass existingOrgs length into loader params now that th…
theohh0 806e348
added select/delete org permission test
theohh0 3170684
enable organizations in test realm
theohh0 90d9f30
reusable selectedRows var
theohh0 25897c0
hopefully more concrete org row selector
theohh0 c39f92a
create policy with different name
theohh0 cc35527
just check for presence of data property - not truthiness
theohh0 8430850
clean up perm tests once complete
theohh0 2caccc3
Merge branch 'select-org-fgap2' of https://github.com/theohh0/keycloa…
theohh0 3e53d55
Make sure delete completes
theohh0 a86836d
disable row selection only when var exists and true in KeycloakDataTable
theohh0 d83e94f
corrected copilots edit
theohh0 97fd06f
Merge branch 'main' into select-org-fgap2
theohh0 930f35c
Handle undefined organization in findOne callers
UCNDanny 474e96e
use isRadio to manage evaluation 'canSelectAll' so column headings do…
theohh0 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
169 changes: 169 additions & 0 deletions
169
js/apps/admin-ui/src/permissions-configuration/resource-types/OrganizationSelect.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| import { | ||
| FormErrorText, | ||
| HelpItem, | ||
| useFetch, | ||
| } from "@keycloak/keycloak-ui-shared"; | ||
|
|
||
| import { useTranslation } from "react-i18next"; | ||
| import { Button, FormGroup } from "@patternfly/react-core"; | ||
| import OrganizationRepresentation from "@keycloak/keycloak-admin-client/lib/defs/organizationRepresentation"; | ||
| import { useState } from "react"; | ||
| import { Controller, useFormContext } from "react-hook-form"; | ||
| import { Table, Tbody, Td, Th, Thead, Tr } from "@patternfly/react-table"; | ||
| import { MinusCircleIcon } from "@patternfly/react-icons"; | ||
| import { ComponentProps } from "../../components/dynamic/components"; | ||
| import { useAdminClient } from "../../admin-client"; | ||
| import { OrganizationModal } from "../../organizations/OrganizationModal"; | ||
|
|
||
| type OrganizationSelectProps = Omit<ComponentProps, "convertToName"> & { | ||
| variant?: "typeahead" | "typeaheadMulti"; | ||
| isRequired?: boolean; | ||
| isRadio?: boolean; | ||
| }; | ||
|
|
||
| const convertOrganizations = ( | ||
| organizations: OrganizationRepresentation[], | ||
| ): string[] => organizations.map(({ id }) => id!); | ||
|
|
||
| export const OrganizationSelect = ({ | ||
|
theohh0 marked this conversation as resolved.
Resolved
|
||
| label, | ||
| helpText, | ||
| defaultValue, | ||
| isDisabled = false, | ||
| isRequired, | ||
| name, | ||
| variant = "typeaheadMulti", | ||
| isRadio = false, | ||
| }: OrganizationSelectProps) => { | ||
| const { adminClient } = useAdminClient(); | ||
| const { t } = useTranslation(); | ||
| const { | ||
| control, | ||
| setValue, | ||
| getValues, | ||
| formState: { errors }, | ||
| } = useFormContext(); | ||
|
|
||
| const values: string[] = getValues(name!) || []; | ||
|
theohh0 marked this conversation as resolved.
Resolved
theohh0 marked this conversation as resolved.
Resolved
theohh0 marked this conversation as resolved.
Resolved
|
||
| const [open, setOpen] = useState(false); | ||
| const [organizations, setOrganizations] = useState< | ||
| OrganizationRepresentation[] | ||
| >([]); | ||
|
|
||
| useFetch( | ||
| () => { | ||
| if (values.length > 0) { | ||
| return Promise.all( | ||
| (values as string[]).map((id) => | ||
| adminClient.organizations.findOne({ id }), | ||
| ), | ||
| ); | ||
| } | ||
| return Promise.resolve([]); | ||
| }, | ||
| (organizations) => { | ||
| setOrganizations( | ||
| organizations.filter((o) => o) as OrganizationRepresentation[], | ||
| ); | ||
| }, | ||
| [], | ||
| ); | ||
|
theohh0 marked this conversation as resolved.
Resolved
theohh0 marked this conversation as resolved.
Resolved
theohh0 marked this conversation as resolved.
Resolved
theohh0 marked this conversation as resolved.
Resolved
theohh0 marked this conversation as resolved.
Resolved
theohh0 marked this conversation as resolved.
Resolved
|
||
|
|
||
| const selectOne = variant === "typeahead"; | ||
|
|
||
| return ( | ||
| <FormGroup | ||
| label={t(label!)} | ||
| labelIcon={ | ||
| <HelpItem helpText={t(helpText!)} fieldLabelId="organizations" /> | ||
| } | ||
| fieldId="organizations" | ||
|
theohh0 marked this conversation as resolved.
Resolved
theohh0 marked this conversation as resolved.
Resolved
|
||
| isRequired={isRequired} | ||
| > | ||
| <Controller | ||
| name={name!} | ||
| control={control} | ||
| defaultValue={defaultValue} | ||
| rules={{ | ||
| validate: (value?: string[]) => | ||
| !isRequired || (value && value.length > 0), | ||
| }} | ||
| render={({ field }) => ( | ||
| <> | ||
| {open && ( | ||
| <OrganizationModal | ||
|
ssilvert marked this conversation as resolved.
Resolved
|
||
| mode="add" | ||
| existingOrgs={organizations} | ||
| => setOpen(false)} | ||
| (orgs) => { | ||
| // not actually async but onAdd is called assuming it is | ||
| if (selectOne) { | ||
|
theohh0 marked this conversation as resolved.
Resolved
|
||
| field.onChange(convertOrganizations(orgs)); | ||
| setOrganizations(orgs); | ||
| } else { | ||
| field.onChange([ | ||
| ...(field.value || []), | ||
| ...convertOrganizations(orgs), | ||
| ]); | ||
| setOrganizations([...organizations, ...orgs]); | ||
| } | ||
| setOpen(false); | ||
| }} | ||
| isRadio={isRadio} | ||
| /> | ||
| )} | ||
| <Button | ||
| data-testid="select-organization-button" | ||
| isDisabled={isDisabled} | ||
| variant="secondary" | ||
| => { | ||
| setOpen(true); | ||
| }} | ||
| > | ||
| {t("addOrganizations")} | ||
| </Button> | ||
| </> | ||
| )} | ||
| /> | ||
| {organizations.length > 0 && ( | ||
| <Table variant="compact"> | ||
| <Thead> | ||
| <Tr> | ||
| <Th>{t("organizations")}</Th> | ||
| <Th aria-hidden="true" /> | ||
| </Tr> | ||
| </Thead> | ||
| <Tbody> | ||
| {organizations.map((organization) => ( | ||
| <Tr key={organization.id}> | ||
| <Td>{organization.name}</Td> | ||
| <Td> | ||
| <Button | ||
| variant="link" | ||
| className="keycloak__client-authorization__policy-row-remove" | ||
| icon={<MinusCircleIcon />} | ||
| => { | ||
| setValue(name!, [ | ||
|
ssilvert marked this conversation as resolved.
Resolved
|
||
| ...convertOrganizations( | ||
| organizations.filter( | ||
| ({ id }) => id !== organization.id, | ||
| ), | ||
| ), | ||
| ]); | ||
| setOrganizations([ | ||
| ...organizations.filter( | ||
| ({ id }) => id !== organization.id, | ||
| ), | ||
| ]); | ||
| }} | ||
| /> | ||
|
theohh0 marked this conversation as resolved.
Resolved
theohh0 marked this conversation as resolved.
Resolved
theohh0 marked this conversation as resolved.
Resolved
|
||
| </Td> | ||
| </Tr> | ||
| ))} | ||
| </Tbody> | ||
| </Table> | ||
| )} | ||
| {errors[name!] && <FormErrorText message={t("requiredOrganizations")} />} | ||
| </FormGroup> | ||
| ); | ||
| }; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Casing and placement are sorted now, thanks. Two key issues left, both making the UI show raw keys:
selectOrganizationis never read, and the key that is read doesn't exist. The evaluation tab builds the help key ast(`select${selectedResourceType}`)(PermissionsEvaluationTab.tsx:203), and the resource type isOrganizations, so it looks upselectOrganizations— plural. Only the singular is defined here, so the popover renders the literal stringselectOrganizations. Either add the plural key, or point the modal title at the singular one (see my reply on theTITLESthread, which would give the singular a consumer).requiredOrganizationsis missing.OrganizationSelect.tsx:164rendersFormErrorTextwitht("requiredOrganizations")and there's no such key, so pedroigor's validation message — the thing this PR addedFormErrorTextfor — currently displays asrequiredOrganizations.On your question in the PR thread about
requiredGroups: agreed, that one is pre-existing and a separate PR is the right call.requiredOrganizationsis different though — it's new in this PR and it's the only thing standing between the validation fix and it actually working, so it should land here. For what it's worthselectRolesis missing too, so the plural-key convention is already patchy inmain; worth folding into the same follow-up asrequiredGroupsif you pick it up.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add the plural key for selectOrganizations but I would note that perhaps (for all the resource types) using the plural doesnt make sense (i think some say just "select user" but others like clients say "select clients") as for the permission evaluation you are only selecting one resource.
Also in checking this I've noticed a bug (which I've verified is on main and not just my branch) where if you select the roles resource when evaluating permissions, it crashes saying "Cannot read properties of undefined (reading 'length')"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Singular or plural is fine for the text — just use what you think makes sense. Note the key name itself isn't optional though:
PermissionsEvaluationTab.tsx:200doest(`select${selectedResourceType}`), so it has to beselectOrganizationseven if the text reads "Select organization".selectUsers=Select useralready does exactly that.We mainly don't want to have "not found" localization keys. On that note —
selectRolesis missing entirely, so Roles already shows a raw key on main. Worth rolling into the same fix as the Roles evaluation crash you found.