[
https://issues.apache.org/jira/browse/OAK-4845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15524062#comment-15524062
]
Alexander Klimetschek commented on OAK-4845:
--------------------------------------------
I noticed that a lost membership is not applied in such a local group
membership either. This was always true, even before OAK-4397.
First the [isSameIDP() check inside
syncMemberships()|https://github.com/apache/jackrabbit-oak/blob/8c928feafc77c40d75142f69931b6ff3b5837f9d/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContext.java#L480-L482]
fills the {{declaredExternalGroups}} map and then later the [removal of lost
memberships|https://github.com/apache/jackrabbit-oak/blob/8c928feafc77c40d75142f69931b6ff3b5837f9d/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContext.java#L535-L536]
is only done against this list/map (code links are oak 1.4.2, but current
trunk is identical). Hence any group that existed locally before, would not get
a {{rep:externalId}}, and hence later on membership removal it would get
ignored.
I see the need that you want to make sure a membership is not removed
accidentally if it "additionally" comes in from another IDP. OAK-4397
definitely made the entire behavior more consistent, in that you can't even add
such memberships in the first place.
However, from all I can see in our use case we have no other option than have
this group exist locally before (and definitely there are production systems
with this state already). IMO it should be ok to say a "local only group" with
no {{rep:externalId}} gets upgraded to an IDP group by adding the
{{rep:externalId}} the first time it is synced - and from then on all the
checks stay in place, to prevent another IDP to mess with it. This would be a
slightly different patch.
> Regression: DefaultSyncContext does not sync membership to a local group
> ------------------------------------------------------------------------
>
> Key: OAK-4845
> URL: https://issues.apache.org/jira/browse/OAK-4845
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: auth-external
> Affects Versions: 1.5.3, 1.4.7
> Reporter: Alexander Klimetschek
> Priority: Critical
> Attachments: OAK-4845.patch, missing-commits-in-tests.patch
>
>
> OAK-4397 introduced a regression: it does not allow syncing to a locally
> existing group anymore (that does not belong to another IDP).
> Updating to 1.4.7 now gives "Existing authorizable 'X' is not a group from
> this IDP 'foo'.", and the group is not synced and most importantly,
> memberships for external users are not updated anymore. Looking at the group
> in JCR, it does not have a {{rep:externalId}} at all, only a
> {{rep:lastSynced}}.
> Code wise, this is because the {{rep:externalId}} is only ever set in
> {{createGroup()}}, i.e. when the external sync creates that group initially.
> If the group is already present locally (but not owned by another IDP!), then
> it won't use it due to the new {{isSameIDP()}} check, which will also fail if
> there is no {{rep:externalId}} property set.
> The use case is that we are defining the group locally as part of our
> application already (including ACs etc.) and we want certain external users
> be added to it, based on some some of their settings on the external IDP
> side. FWIW, we don't care for the syncing of properties for the group itself,
> just for the memberships.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
