8000 Resolve realm display name localization placeholders in TOTP issuer name by mabagoury · Pull Request #48700 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Resolve realm display name localization placeholders in TOTP issuer name - #48700

Merged
rmartinc merged 2 commits into
keycloak:mainfrom
mabagoury:fix/totp-issuer-name-localization
May 12, 2026
Merged

Resolve realm display name localization placeholders in TOTP issuer name#48700
rmartinc merged 2 commits into
keycloak:mainfrom
mabagoury:fix/totp-issuer-name-localization

Conversation

@mabagoury
@mabagoury mabagoury commented May 4, 2026
Copy link
Copy Markdown
Contributor

Resolve the realm display name before it is used as the issuer name when configuring TOTP credentials. This allows values that use localization placeholder syntax (e.g., ${realm.displayName}) to be resolved using the LOGIN theme messages and the user’s resolved locale, so authenticator applications show the localized display name instead of the raw placeholder.

Fixes #48680

@mabagoury
mabagoury requested a review from a team as a code owner May 4, 2026 23:12
@mabagoury
mabagoury force-pushed the fix/totp-issuer-name-localization branch from c27aa78 to 4f0f206 Compare May 4, 2026 23:21
@mabagoury
mabagoury commented May 4, 2026
Copy link
Copy Markdown
Contributor Author

I kept issuer resolution in services/.../TotpUtils.java instead of server-spi/.../OTPPolicy.java.

OTPPolicy is a suitable place for building the TOTP URI. However, resolving the realm display name requires:

  • LOGIN theme message bundles
  • the user’s resolved locale
  • theme/templating utilities (e.g., TemplatingUtil)

These dependencies belong to the service layer rather than the SPI layer. As a result, TotpUtils resolves the issuer string first and then delegates to OTPPolicy#getKeyURI(String issuer, String user, String secret).

Additionally, OTPPolicy#getKeyURI(RealmModel, UserModel, String) appears unused within this repository. Only TotpUtils used it. We could consider removing the unused overload, provided there is no external usage, since it is part of the public SPI surface and may have external consumers.

@keycloak-github-bot
keycloak-github-bot Bot commented May 5, 2026
Copy link
Copy Markdown

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.forms.BruteForceTest#testNoFailureResetForPermanentLockout

Keycloak CI - Base IT (5)

org.opentest4j.AssertionFailedError: Expected error event ==> expected: <true> but was: <false>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
...

Report flaky test

keycloak-github-bot[bot]
keycloak-github-bot Bot reviewed May 5, 2026
View reviewed changes
@keycloak-github-bot keycloak-github-bot Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unreported flaky test detected, please review

@mabagoury
mabagoury force-pushed the fix/totp-issuer-name-localization branch from 4f0f206 to a3a19f4 Compare May 5, 2026 20:36
mabagoury and others added 2 commits May 7, 2026 15:39
@mabagoury @mabartos
Resolve realm display name localization placeholders in TOTP issuer name
c7a3b42 
Resolve the realm display name before it is used as the issuer name when configuring TOTP credentials. This allows values that use localization placeholder syntax (e.g., ${realm.displayName}) to be resolved using the LOGIN theme messages and the user’s resolved locale, so authenticator applications show the localized display name instead of the raw placeholder.

Fixes keycloak#48680

Signed-off-by: Mohammed Elbagoury <m.a.bagoury@gmail.com>
@mabartos
Cleanup the TotpUtils + add deprecation
ea80308 
Signed-off-by: Martin Bartoš <mabartos@redhat.com>
@mabartos
mabartos force-pushed the fix/totp-issuer-name-localization branch from a3a19f4 to ea80308 Compare May 7, 2026 13:39
mabartos
mabartos approved these changes May 7, 2026
View reviewed changes
@mabartos mabartos left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just polished it a little. LGTM, thanks!

@mabartos
mabartos requested a review from rmartinc May 12, 2026 06:46
@mabartos
mabartos commented May 12, 2026
Copy link
Copy Markdown
Member

@rmartinc Could you please check this little PR? Thanks!

I was able to reproduce it and the fix works now.

@rmartinc rmartinc self-assigned this May 12, 2026
rmartinc
rmartinc approved these changes May 12, 2026
View reviewed changes
@rmartinc rmartinc left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @mabagoury! LGTM!

@rmartinc
rmartinc merged commit 77bf9a1 into keycloak:main May 12, 2026
85 checks passed
@mabagoury
mabagoury deleted the fix/totp-issuer-name-localization branch May 12, 2026 17:06
@stianst stianst mentioned this pull request Jun 1, 2026
Closed
sashyo pushed a commit to tide-foundation/keycloak-IGA that referenced this pull request Jul 20, 2026
@mabagoury @mabartos @sashyo
Resolve realm display name localization placeholders in TOTP issuer n…
a2e9c96 
…ame (keycloak#48700)

* Resolve realm display name localization placeholders in TOTP issuer name

Resolve the realm display name before it is used as the issuer name when configuring TOTP credentials. This allows values that use localization placeholder syntax (e.g., ${realm.displayName}) to be resolved using the LOGIN theme messages and the user’s resolved locale, so authenticator applications show the localized display name instead of the raw placeholder.

Fixes keycloak#48680

Signed-off-by: Mohammed Elbagoury <m.a.bagoury@gmail.com>

* Cleanup the TotpUtils + add deprecation

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

---------

Signed-off-by: Mohammed Elbagoury <m.a.bagoury@gmail.com>
Signed-off-by: Martin Bartoš <mabartos@redhat.com>
Co-authored-by: Martin Bartoš <mabartos@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rmartinc rmartinc rmartinc approved these changes
@mabartos mabartos mabartos approved these changes
+1 more reviewer
@keycloak-github-bot keycloak-github-bot[bot] keycloak-github-bot[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@rmartinc rmartinc

Labels

flaky-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Realm display name localization placeholder is not expanded when resolving OTP issuer name

3 participants

@mabagoury @mabartos @rmartinc
0