[
https://issues.apache.org/jira/browse/OAK-4932?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Alexander Klimetschek updated OAK-4932:
---------------------------------------
Description:
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.
was:
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.
> 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)
