Repository navigation
Conversation
edewit
force-pushed
the
storage-type-declarative-ui
branch
from
June 29, 2026 09:17
e69d211 to
fabcd85
Compare
Contributor
Author
|
@ssilvert would this be something we could add? |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the declarative Admin UI extension mechanism to support alternative persistence backends beyond Keycloak Components by introducing a storageType (and optional custom endpoint) in provider metadata, and updating the Admin UI to load/save configuration accordingly.
Changes:
- Add
StorageTypeand exposestorageType(and optionalendpoint) viaUiTabProviderFactory#getTypeMetadata(). - Update Admin UI
PageHandlerto fetch and persist tab/page config to different entity types (client/user/group/IdP) or to a custom REST endpoint.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| server-spi-private/src/main/java/org/keycloak/services/ui/extend/UiTabProviderFactory.java | Exposes storageType and optional endpoint in UI extension metadata; adds new defaults and Javadoc. |
| server-spi-private/src/main/java/org/keycloak/services/ui/extend/StorageType.java | Introduces a new enum describing supported storage backends for UI tab data. |
| js/apps/admin-ui/src/page/PageHandler.tsx | Adds storage-type-specific fetch/save logic, including custom endpoint GET/PUT support. |
edewit
force-pushed
the
storage-type-declarative-ui
branch
from
June 29, 2026 10:17
6e0d413 to
3ee3a56
Compare
Contributor
Author
|
@ssilvert can you have a look? |
edewit
force-pushed
the
storage-type-declarative-ui
branch
from
July 10, 2026 08:28
17fe945 to
2054da4
Compare
Contributor
|
@edewit I'm going to wait until the Copilot comments are resolved one way or another before I do a review. Just let me know when you are ready. |
Contributor
Author
|
@ssilvert All Copilot review threads have been resolved. Summary of changes addressing the valid feedback:
Previously reported issues (REALM javadoc, useForm typing, syntax errors, validation placement) were already fixed in earlier commits. Ready for your review when you have a chance. |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Three unresolved moderate findings affect component discovery and permission consistency.
Review details
Suppressed comments (3)
services/src/main/java/org/keycloak/services/resources/admin/ComponentResource.java:129
- When
typeis queried without aproviderId, this branch selects onlycustomComponentsas soon as any custom-storage factory exists. That silently drops all realm-backed components of the same type whenever custom and default providers are installed together; this path should fall through to the merge below so both backends are returned.
} else if (customComponents != null && type != null && providerId == null
&& !UiExtensionComponentStorage.getCustomStorageProviderIds(session, type).isEmpty()) {
components = customComponents;
services/src/main/java/org/keycloak/services/resources/admin/UiExtensionComponentStorage.java:59
- With neither
typenorproviderId, this returnsnull, causingComponentResourceto use onlyrealm.getComponentsStream()and omit every component held by aComponentStorageFactory. Consequently custom-storage components are not discoverable through the standard unfilteredGET /componentsendpoint, unlike realm-backed components; enumerate the applicable custom UI storage factories for this query as well.
if (type == null) {
return null;
services/src/main/java/org/keycloak/services/resources/admin/UiExtensionPermissions.java:20
- The UI treats an empty
requiredViewRolesorrequiredManageRolesmetadata list as the default realm role, andrequireManagedoes the same, butrequireViewcallshasAnyRoledirectly on empty lists. A provider overriding either method withList.of()can therefore be displayed by the UI while its view/config/component requests are rejected with 403; apply the same default-role fallback here.
static void requireView(AdminPermissionEvaluator auth, UiExtensionSupport extension) {
if (hasAnyRole(auth, extension.getRequiredViewRoles())
|| hasAnyRole(auth, extension.getRequiredManageRoles())) {
return;
}
throw new ForbiddenException();
- Files reviewed: 27/27 changed files
- Comments generated: 0 new
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Component queries omit valid records, form handling breaks multivalued editing and runtime-property persistence, and inconsistent role defaults deny reads to users permitted to edit.
5 open issues
5 resolved since last review
ProviderConfigProperty.type),…nullresult fromupdateComponentis…idis local state and can be populated after creating a page, but this reset effect does not…
Review effort: Balanced
| (data) => { | ||
| form.reset({}); | ||
| if (data) { | ||
| convertToFormValues(data, form.setValue); |
Comment on lines
+39
to
+41
| default List<ProviderConfigProperty> getConfigProperties(KeycloakSession session, Map<String, String> contextParams) { | ||
| if (this instanceof ConfiguredProvider configuredProvider) { | ||
| return configuredProvider.getConfigProperties(); |
Comment on lines
+136
to
+139
| } else if (customComponents != null && type != null && providerId == null | ||
| && !UiExtensionComponentStorage.getCustomStorageProviderIds(session, type).isEmpty()) { | ||
| components = customComponents; | ||
| } else if (customComponents != null) { |
Comment on lines
+58
to
+59
| if (type == null) { | ||
| return null; |
Comment on lines
+16
to
+17
| if (hasAnyRole(auth, extension.getRequiredViewRoles()) | ||
| || hasAnyRole(auth, extension.getRequiredManageRoles())) { |
By adding a storage type to your UI it can save the config onto other models and bypass the update and create logic from your provider Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Scope normalizeConfig to declared properties only, document the CUSTOM endpoint protocol, and add unit tests for pageHandlerStorage helpers. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Treat IdentityProviderMultiList as multivalued for string-map round-trips and delete declared properties when form controls clear their values. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Guard against missing client, user, and identity provider resources before narrowing types used by mergeEntityConfig. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Add runtime config endpoint, FGAP-aware permissions, navigation sections, tab i18n, page detail tabs, ComponentStorageFactory SPI, admin guide, and promote DECLARATIVE_UI to preview. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Fix permission checks, runtime config sync, dotted property form mapping, custom component storage lookup, realm URL encoding, and route access for extension pages with custom roles. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Pass all runtime config query params, gate page mutations by manage roles, show extension nav when only extension pages are visible, filter internal custom storage factories, and treat empty multiline values as cleared. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Revalidate component provider after update, block read-only form submits, document custom endpoint authorization, clarify detailTabPath contract, and use detailTabPath in page list links. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Introduce a test UiPageProviderFactory with custom in-memory storage and verify ComponentResource CRUD delegates outside the realm component table when ComponentStorageFactory is implemented. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Aggregate custom-storage components in type-only queries, reject provider changes across custom-storage boundaries, and skip page fetches for users without view access. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Check realm permissions before returning 404 for missing components, return internal components for update/delete so forbidden is returned instead of not found, validate custom-storage provider changes before model update, and resolve custom storage factories by provider ID alone. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Reset tab component IDs on route changes, avoid findOne for tab-backed loads, filter type-wide component queries per factory permissions, and treat manage roles as sufficient for view access in the declarative UI. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Exclude realm component entries for providers that delegate to ComponentStorageFactory when merging with custom-storage results so type-only component queries do not double-count the same entries. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Merge realm and custom-storage component streams by ID so UI extension components are not listed twice when both stores expose the same entry. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
When a provider type is backed only by custom storage factories, return the custom component stream directly and never persist those components to the realm component table on create. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Filter realm components by custom-storage provider IDs and deduplicate custom-storage listings by component ID so type-wide queries return a single entry for custom-storage UI page components. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
List custom-storage components without merging realm entries when the provider type is fully custom-backed, and clean up UiPageProvider components between test methods to avoid static storage leakage. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
The previous commit referenced this method in @BeforeEach/@AfterEach but never added the implementation, which broke the build. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Replace normalizeConfig with pickDeclaredConfig, convertToFormValues, stringify on DynamicComponents, and a slimmer mergeEntityConfig. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
findOne can return null, which is incompatible with useState's setter type and broke the admin-ui TypeScript build. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Unresolved defects omit stored components from queries, silently lose runtime-defined fields, break multivalued inputs, and incorrectly deny realm-admin reads.
5 open issues
Review effort: Balanced
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved defects omit components from listings, prevent runtime-defined fields from loading and saving, and break form restoration, multivalued inputs, and permission fallbacks.
6 open issues
Review effort: Balanced
Comment on lines
+259
to
+262
| form.reset({}); | ||
| if (data) { | ||
| convertToFormValues(data, form.setValue); | ||
| } |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
By adding a storage type to your UI it can save the config onto other
models and bypass the update and create logic from your provider
fixes: #47858
Signed-off-by: Erik Jan de Wit erikjan.dewit@gmail.com