Skip to content
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

not able to disable declarative_ui feature #29057

Closed
1 of 2 tasks
avaraljai opened this issue Apr 24, 2024 · 4 comments · Fixed by #29101
Closed
1 of 2 tasks

not able to disable declarative_ui feature #29057

avaraljai opened this issue Apr 24, 2024 · 4 comments · Fixed by #29101

Comments

@avaraljai
Copy link

Before reporting an issue

  • I have read and understood the above terms for submitting issues, and I understand that my issue may be closed without action if I do not follow them.

Area

admin/ui

Describe the bug

The DECLARATIVE_UI feature extendable admin console could not be turned off.

Referring to this recently added feature:
#23772

I was trying out the introduced feature based on this quickstart:
https://github.com/keycloak/keycloak-quickstarts/tree/latest/extension/extend-admin-console-spi

once i have the extend-admin-ui,jar in the providers folder the attempt to disable the extension seems failing.
see the attached screenshots one with enabled one with disabled. ("todo" item is visible on the left regardless of the setting)

Maybe i got it wrong but i thought that it is possible to switch off this experimental feature.

2024-04-24_16-10_declarative_ui_enabled
2024-04-24_16-11_declarative_ui_disabled

Version

24.0.3

Regression

  • The issue is a regression

Expected behavior

when i set declarative_ui disabled the UI elements should not be visible

Actual behavior

when i set declarative_ui disabled the UI elements are visible

How to Reproduce?

deploy the above mentioned quickstart ( https://github.com/keycloak/keycloak-quickstarts/tree/latest/extension/extend-admin-console-spi )

to disable i added the "declarative_ui" to this envvar KC_FEATURES_DISABLED

then kc.sh build

Anything else?

No response

@stianst
Copy link
Contributor

stianst commented Apr 25, 2024

@jonkoops this seems to be a admin ui issue to me, and not a core issue?

@avaraljai
Copy link
Author

Well i'm not aware of the boundaries of the project, what i could figure maybe the way of applying the feature flag is not ok:
https://github.com/keycloak/keycloak/pull/23772/files#diff-b85fcd54076f5d287313bfb7494f1f0176a61044fbf7fcd6e42480ddaf3063a2R30

https://github.com/keycloak/keycloak/pull/23772/files#diff-04fee0492694816f36c68a468a9ec735283d006b5e6f19502ea8109c4723d646R30

both relying on SPI.isEnabled()

I found 1 occurence of checking spi.isEnabled() in org.keycloak.services.DefaultKeycloakSessionFactory#init

and org.keycloak.quarkus.runtime.integration.QuarkusKeycloakSessionFactory#init


overrides this so i believe spi.isEnabled() not being checked when QuarkusKeycloakSessionFactory is being used

But i haven't checked this with debugger.
So maybe SPI.isEnabled() is not the right way to apply the feature gate on this functionality, or the QuarkusKeycloakSessionFactory does not check this SPI.isEnabled()

Or maybe something else - please feel free to validate my assumptions.

@keycloak-github-bot
Copy link

Due to the amount of issues reported by the community we are not able to prioritise resolving this issue at the moment.

If you are affected by this issue, upvote it by adding a 👍 to the description. We would also welcome a contribution to fix the issue.

@jonkoops
Copy link
Contributor

Hmmm, yeah looks like a missing condition somewhere. @edewit could you take a look at this?

edewit added a commit to edewit/keycloak that referenced this issue Apr 26, 2024
fixes: keycloak#29057

Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
edewit added a commit that referenced this issue Apr 30, 2024
fixes: #29057

Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants