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

Julian Sedding commented on SLING-6285:
---------------------------------------

[~bdelacretaz], your observation about 
{{AbstractSlingRepositoryManager.isAllowLoginAdministrativeForBundleOverridden}}
 is correct, it is not required. However, unfortunately this interface change 
has is already  released in version 2.4.2. So removing it would require a major 
version bump IIUC, which I wanted to avoid. We can deprecate it of course and 
remove or make it private later on.

Having the possibility to override 
{{isAllowLoginAdministrativeForBundleOverridden}} allows implementations 
extending from the abstract base classes to implement a different strategy for 
whitelisting. So far this was only helpful for unit testing, see 
{{MockSlingRepositoryManager}}.

I chose to use reflection, because that allowed me to only bind the 
{{LoginAdminWhitelist}} if the subclass inherits 
{{isAllowLoginAdministrativeForBundleOverridden}}. Note also that the 
reflection calls happen exactly once during {{activate}}.

We can introduce another method to indicate that we indeed want to use the base 
whitelist implementation, however, this is error prone, because implementations 
have to turn two screws in sync: they need to indicate whether or not they 
override {{isAllowLoginAdministrativeForBundleOverridden}}.

Maybe the best thing is to deprecate 
{{isAllowLoginAdministrativeForBundleOverridden}} now, which allows us to 
remove the reflection based code once we remove the method.

> Implement LoginAdminWhitelist in JCR Base
> -----------------------------------------
>
>                 Key: SLING-6285
>                 URL: https://issues.apache.org/jira/browse/SLING-6285
>             Project: Sling
>          Issue Type: Improvement
>          Components: JCR
>    Affects Versions: JCR Base 2.4.2
>            Reporter: Julian Sedding
>            Assignee: Julian Sedding
>             Fix For: JCR Oak Server 1.1.4, JCR Base 3.0.0
>
>         Attachments: SLING-6285-jsedding.patch
>
>
> JCR Base should provide a default implementation 
> {{AbstractSlingRepositoryManager#allowLoginAdministrativeForBundle(Bundle)}} 
> with a configurable {{LoginAdminWhitelist}}. Implementation that want a 
> different logic can then still choose to overwrite the method.
> cc [~cziegeler], [~bdelacretaz]



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

Reply via email to