Alexander Klimetschek created OAK-4932:
------------------------------------------
Summary: 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
(v6.3.4#6332)
