Alexander Klimetschek commented on OAK-4932:

/cc [~tripod] [~djaeggi] [~anchela]

> 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 use 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

Reply via email to