search

p2-inc / keycloak-themes

Themes and theme utilities meant for simple theme customization without deploying a packaged theme
https://phasetwo.io
Other
28 stars 11 forks source link

Setting Email theme only works for master realm. #6

Closed sweeneytr closed 1 year ago

sweeneytr commented 1 year ago

Setting the email theme correctly writes to the realm attributes per-realm, but only the email attributes on the root realm are used when sending emails, or when loading the UI.

xgp commented 1 year ago

@sweeneytr Thanks for the report. Quick clarification, are you setting the attributes (or using the UI) while logged in as a master realm administrator? I can't reproduce when I'm just logged in as a non-master realm admin.

Also, I'm not sure what you mean by "when loading the UI". Can you be more specific?

sweeneytr commented 1 year ago

This is occurring when using the UI. We're seeing the attributes being set correctly in the realm attributes when saving the UI, but when reloading the UI the settings appear to be reset, despite being persisted to the realm attributes successfully.

sweeneytr commented 1 year ago

What I meant by "loading via the UI" was that when opening the UI to edit the email settings, the html template is reverting to the default. This is at odds with what's actually set in the realm attributes

xgp commented 1 year ago

Thanks for the clarification. Would you mind doing a quick screen capture to show me?

sweeneytr commented 1 year ago

Sure, this is the example of "attribute is set for the realm, but isn't used for that realm.

Screencast from 05-09-2023 02:23:50 PM.webm

xgp commented 1 year ago

Perfect. Thank you. Can you confirm that you're running kc 21.1.1? We've seen some strange behaviors with emails in that version, and there might be an incompatibility.

xgp commented 1 year ago

I'm wondering if it could be some change in theme caching behavior. Can you try running with --spi-theme-static-max-age=-1 --spi-theme-cache-themes=false --spi-theme-cache-templates=false to see if that clears it?

The test here https://github.com/p2-inc/keycloak-themes/blob/main/src/test/java/io/phasetwo/keycloak/themes/resource/EmailsResourceTest.java#L80 covers this case, though this test is actually getting run with the legacy org.keycloak.testsuite.KeycloakServer. I'll try to port the test to 21.1.1 on testcontainers to see if that makes it repeatable.

sweeneytr commented 1 year ago

Can you try running with --spi-theme-static-max-age=-1 --spi-theme-cache-themes=false --spi-theme-cache-templates=false to see if that clears it?

I'm not able to manage the orchestration of this that tightly, but I can tell you it persists as an issue between restarts of the service.

We're running on 21.1.1.1683733514

xgp commented 1 year ago

Thanks. Still looking into this one.

I can confirm that there is a problem when setting these logged in to the master realm. It appears that there is an issue using session.getContext().getRealm() subsequently, where even when using from another realm, it returns the master realm values.

However, I'm not able to reproduce when logged into another realm. Seems to work properly:

https://github.com/p2-inc/keycloak-themes/assets/244253/61bc82bb-9df5-425d-a9ec-d4c3be1bdeb1

sweeneytr commented 1 year ago

when setting these logged in to the master realm

That'd be it. We are logging into the master realm to do administration.

sweeneytr commented 1 year ago

We're able to work around this by setting the themes on the master realm, so it's not high priority from our PoV to get it fixed. Don't sweat over it

xgp commented 1 year ago

This will fix it. https://github.com/p2-inc/keycloak-themes/pull/8 Couple of things.

  1. I don't think the workaround you mentioned is stable. The way themes are cached in Keycloak, it will essentially cache the first time the theme is loaded, which keeps the KeycloakSession object in that Theme object. Which means that a call to the foo realm for the theme, will continue to return the templates from the foo realm attributes, even if you are trying to use it from the bar realm.
  2. In order to make the above PR work, you will need to set the --spi-theme-cache-themes=false startup flag. This will stop the caching of all themes. I'm looking for a way to only do it for this theme alone, but I haven't found one yet. I can't really tell if there is a performance impact, but there may be (frankly, looking at the Keycloak theme caching code, it doesn't actually appear to solve a real problem).
  3. The ability to get/set templates in another, non-master realm, when logged into the master realm works now.