Repository navigation
Fixed: externalLoginKey enable flag never checked, cannot be changed live (OFBIZ-13519) - #1848
Merged
mridulpathak merged 1 commit intoSep 3, 2026
Conversation
…ExternalLoginKey() and cannot be changed live (OFBIZ-13519) isExternalLoginKeyEnabled() cached the property in a static field on first read and never re-read it, so any later change made no difference until a process restart. Separately, checkExternalLoginKey() -- the consumption path wired into the default preprocessor chain used by most controller.xml files -- never called isExternalLoginKeyEnabled() at all; the flag was only ever checked at the two key-generation call sites. Together this meant an admin disabling the property got no change in behavior on either the caching or the enforcement side. Removed the permanent caching so the property is re-read on every call, and added the missing check to checkExternalLoginKey() so it short-circuits to success without touching the key when the feature is disabled.
Contributor
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.
Backported from trunk (#1847). isExternalLoginKeyEnabled() cached the security.login.externalLoginKey.enabled property in a static field on first read and never re-read it, so any later change made no difference until a process restart. Separately, checkExternalLoginKey() -- the consumption path wired into the default preprocessor chain used by most controller.xml files -- never called isExternalLoginKeyEnabled() at all; the flag was only ever checked at the two key-generation call sites (RequestHandler.makeLink(), ScreenRenderer.java). Together this meant an admin disabling the property got no change in behavior on either the caching or the enforcement side. This removes the permanent caching so the property is re-read on every call, and adds the missing check to checkExternalLoginKey() so it short-circuits to success without touching the key when the feature is disabled. The production fix cherry-picked cleanly; the test file did not, since this branch's ExternalLoginKeysManagerTests predates the externalLoginKey hardening work and has no existing coverage of checkExternalLoginKey() to update, so no test changes were needed here.