Details
Major Description
Steps to reproduce
- start Oak repository in OSGi setup with additional required (custom) services that are passed to various security modules as config parameter such as e.g. RestrictionProvider, UserAuthenticationFactory, AuthorizableNodeName or AuthorizableActionProvider
- verify that the security setup contains the custom configurations
- now, force a re-registration of the SecurityProvider by changing a referenced/required security service, which is not associated with the custom configuration as specified in the initial setup
- once completed any SecurityConfiguration, that is associated with custom configuration params such as the examples listed above will no longer have the corresponding params set.
Finding step by step
- SecurityProviderRegistration waits until all configured required service references have been registered and all non-dynamic references have been resolved.
- Once everything is resolved the SecurityProviderRegistration looks as expected including all configuration parameters
- SecurityProviderRegistration now starts creating a new SecurityProvider instance with all the unary and required module references.
- During this step it also calls initializeConfiguration in order to have the modules populated with additional stuff from the SecurityProviderRegistration and it's here we have IMHO a bug: The initializeConfiguration will push the params from SecurityProviderRegistration to the SecurityConfiguration, while at the same time trying to merge params defined directly on the SecurityConfiguration.
Explanation
In a plain Java setup as it was initial designed for the SecurityProviderImpl: The 'local' params from SecurityConfiguration need to take precedence over those present in SecurityProvider.
However, In our new, pure Osgi setup, where there is no such mixed-param-setup, we would need a mandatory overwrite of e.g. RestrictionProvider (s) or AuthorizableActionProvider (s), because the old values in the SecurityConfiguration had not been provided by it's own config but as a matter of fact refer to the old values of the SecurityProviderRegistration, which got unregistered and thus are stale service references.
Potential Fixes
In any case we must have a unit-test that illustrates the problem and allows us to verify that whatever fix we apply actually addresses the problem. I will try to provide that today.
Variant 1
Looking back my feeling is, that we should have moved all those extra-params that get pushed to the SecurityConfiguration as references to the modules. Not sure if/how that is feasible at the current state without risking too many compatibility issues and regressions.
Variant 2
Since we no longer have a mixed java/osgi setup since the introduction of the SecurityProviderRegistration and removed the OSGi-annotations from the old (now pure java) SecurityProviderImpl, we might consider just changing the following call in SecurityProviderRegistration from:
base.setParameters(ConfigurationParameters.of(parameters, base.getParameters()));
to
base.setParameters(ConfigurationParameters.of(base.getParameters(), parameters));
and thus actually doing what we intend to do: replace the existing entries in the SecurityConfiguration by the new ones.