Mailing-List: contact oak-issues-help@jackrabbit.apache.org; run by ezmlm
Precedence: bulk
Reply-To: oak-dev@jackrabbit.apache.org
Date: Tue, 2 Aug 2016 14:40:20 +0000 (UTC)
From: "angela (JIRA)" <jira@apache.org>
To: oak-issues@jackrabbit.apache.org
Message-ID: <JIRA.12992512.1469521448000.199698.1470148820484@Atlassian.JIRA>
In-Reply-To: <JIRA.12992512.1469521448000@Atlassian.JIRA>
References: <JIRA.12992512.1469521448000@Atlassian.JIRA> <JIRA.12992512.1469521448783@arcas>
Subject: [jira] [Comment Edited] (OAK-4599) SecurityProviderRegistration
 fails to update config param of SecurityConfiguration(s)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Tue, 02 Aug 2016 14:40:22 -0000


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

angela edited comment on OAK-4599 at 8/2/16 2:39 PM:
-----------------------------------------------------

Since I didn't manage to reproduce the issue with the OSGi test setup present in Oak trunk, here an alternative approach:

h5. Test B
Alternative approach to illustrate the described behavior by refactoring the configuration-initialization into a separate class included tests. Tests relying on 'parameters' replacing the entries in the {{ConfigurationBase}} will fail without a fix, which for simplicity is included in the patch as well.

[~frm], what do you think? Maybe you see a way to _properly_ reproduce it with an OSGi setup?


was (Author: anchela):
Since I didn't manage to reproduce the issue with the OSGi test setup present in Oak trunk, here an alternative approach:

h6. Test B
Alternative approach to illustrate the described behavior by refactoring the configuration-initialization into a separate class included tests. Tests relying on 'parameters' replacing the entries in the {{ConfigurationBase}} will fail without a fix, which for simplicity is included in the patch as well.

[~frm], what do you think? Maybe you see a way to _properly_ reproduce it with an 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, OAK-4599_trunk_var2.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)