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

Alexander Klimetschek commented on OAK-4932:
--------------------------------------------

But now that OAK-4301 is fixed, it is safe.

One can add a warning that disabling the protection will disable the protection 
:-) Do we know if there is a good reason for customers to always disable the 
protection?

Last but not least one could make it dependent on the protection what to call 
(and document).

IMO conceptually the local user id belongs into the sync (i.e. what maps from 
external to local), and not in the external IDP, which is just declaratively 
representing the external system.

> DefaultSyncContext.sync(String) could use idp.getIdentity(ExternalIdentityRef)
> ------------------------------------------------------------------------------
>
>                 Key: OAK-4932
>                 URL: https://issues.apache.org/jira/browse/OAK-4932
>             Project: Jackrabbit Oak
>          Issue Type: Improvement
>          Components: auth-external
>            Reporter: Alexander Klimetschek
>
> Instead of 
> [idp.getGroup(id)|https://github.com/apache/jackrabbit-oak/blob/08aadf19a5e6b1bb4ca6687623e06140fb1ec5bc/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContext.java#L291]
>  and 
> [idp.getUser(id)|https://github.com/apache/jackrabbit-oak/blob/08aadf19a5e6b1bb4ca6687623e06140fb1ec5bc/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContext.java#L300],
>  the implementation of the DefaultSyncHandler could use 
> {{ExternalIdentityProvider.getIdentity(ExternalIdentityRef)}}, as it looks up 
> the reference right before (based on the {{rep:externalId}}) and fails if not 
> present.
> h4. Reasoning
> Implementing {{getUser/Group(id)}} in an ExternalIdentityProvider can be 
> difficult, because you need a way to search the external identity system 
> efficiently by the local user id, which might not always be the case, if the 
> external system uses another id and is only optimized for that.
> h4. Consequences
> # The only other place using {{ExternalIdentityProvider.getUser(String)}} is 
> the 
> [ExternalLoginModule|https://github.com/apache/jackrabbit-oak/blob/52f1e9a84324135e6a79678bbf209d03c0d2d77d/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/ExternalLoginModule.java#L217],
>  in case the user is pre-authenticated and does not exist locally yet. 
> However, this is a specific use case that might not apply to all identity 
> providers, in which case they could happily skip implementing this method. A 
> note in the javadoc could clarify this for implementors.
> # {{ExternalIdentityProvider.getGroup(String)}} would then be used in no 
> other place (in the sync code) and could even be deprecated, as I can't 
> imagine another application specific use case for it.



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

Reply via email to