[jira] [Comment Edited] (OAK-4599) SecurityProviderRegistration fails to update config param of SecurityConfiguration(s)

["angela (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/OAK-4599?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15398835#comment-15398835 ] 

angela edited comment on OAK-4599 at 7/29/16 6:51 AM:
------------------------------------------------------

h5. 1 Testing

with the attached groovy tests I was able to reproduce the issue in the Oak 1.2 branch but not in Oak trunk. This means that the mentioned {{setParameters}} call might not be the single root cause for the issue. 
Moving forward we first need to understand, which changes between Oak 1.2 and trunk are responsible for the apparent 'fix' of the issue, which might equally just be disguised by some other changes that affect the OSGi setup.
Once this is completed we can re-evaluated the options for fixing the issue.


was (Author: anchela):
h5. Testing

with the attached groovy tests I was able to reproduce the issue in the Oak 1.2 branch but not in Oak trunk. This means that the mentioned {{setParameters}} call might not be the single root cause for the issue. 
Moving forward we first need to understand, which changes between Oak 1.2 and trunk are responsible for the apparent 'fix' of the issue, which might equally just be disguised by some other changes that affect the OSGi setup 

> SecurityProviderRegistration fails to update config param of SecurityConfiguration(s)
> -------------------------------------------------------------------------------------
>
>                 Key: OAK-4599
>                 URL: https://issues.apache.org/jira/browse/OAK-4599
>             Project: Jackrabbit Oak
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.1.8, 1.2.16, 1.0.32, 1.4.5, 1.5.6
>            Reporter: angela
>            Assignee: angela
>         Attachments: OAK-4599_test_1_2.patch, OAK-4599_test_trunk.patch
>
>
> h4. 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.
> h4. 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}}.
> h4. 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.
> h4. 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.
> h5. 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.
> h5. 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:
> {code}base.setParameters(ConfigurationParameters.of(parameters, base.getParameters()));{code}
> to 
> {code}base.setParameters(ConfigurationParameters.of(base.getParameters(), parameters));{code}
> and thus actually doing what we intend to do: replace the existing entries in the {{SecurityConfiguration}} by the new ones.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)