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

angela commented on OAK-4224:
-----------------------------

I searched for other usages of {{Status.FOREIGN}} and found that the 
corresponding situation in {{sync{String userId)}} doesn't include the 
{{ExternalIdentityReference}} in the {{SyncedIdentity}} passed to the result.

IMHO we should really try to keep things as consistent as possible. If you now 
prefer to have the foreign {{ExternalIdentityRef}} in the {{SyncedIdentity}}, I 
will also adjust the {{sync(String userID)}}: a {{null}} ref would then mean 
that it's not an external user/group and a non-null ref value associated with 
FOREIGN status would allow to identity which foreign IDP it belongs to.

I don't have any strong preference as long as it is consistent.

> DefaultSyncContext.sync(ExternalIdentity) should verify IDP
> -----------------------------------------------------------
>
>                 Key: OAK-4224
>                 URL: https://issues.apache.org/jira/browse/OAK-4224
>             Project: Jackrabbit Oak
>          Issue Type: Bug
>          Components: auth-external
>            Reporter: angela
>            Assignee: angela
>            Priority: Minor
>         Attachments: OAK-4224.patch, OAK-4224_2.patch
>
>
> while writing more test for {{DefaultSyncContext}} i realized that the 
> implementation of {{sync(ExternalIdentity)}} doesn't verify that the given 
> external identity belongs to the same IDP than the one associated with the 
> context instance.
> IMHO this would be needed and useful particularly when multiple IDPs are 
> combined. also, the  {{DefaultSyncContext}} is a public exposed class, I 
> would prefer if it would guard against mixing up sync of external identities 
> from different sources.



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

Reply via email to