You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-issues@jackrabbit.apache.org by "Alexander Klimetschek (JIRA)" <ji...@apache.org> on 2016/09/29 20:26:20 UTC

[jira] [Reopened] (OAK-4845) Regression: DefaultSyncContext does not sync membership to a local group

     [ https://issues.apache.org/jira/browse/OAK-4845?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Alexander Klimetschek reopened OAK-4845:
----------------------------------------

Thanks [~djaeggi]. Unfortunately, the issue extends to removal of lost group memberships, see my previous comment (this was actually always the case with DefaultSyncContext, even before OAK-4397, so that part is not really a regression).

Added a test case for this: [^OAK-4845-test-lostmembership.patch]

I see two solutions:
# switch [line 507|https://github.com/apache/jackrabbit-oak/blob/f9e34556709fcffed5325dcfed8adac53afc9369/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContext.java#L507] to use {{isLocalOrSameIDP()}} as well; this means a local group will never get a {{rep:externalId}} (but still a {{rep:lastSynced}})
# add {{rep:externalId}} to an existing local group inside e.g. {{syncGroup()}} instead of {{createGroup()}} only (this will not fix the test case above, because that never syncs normally first)

The first solution is simple, but has the disadvantage (like the current state) that multiple IDPs could sync to the same local group (since the {{rep:externalId}} is never set), which is something that OAK-4397 wants to prevent and I think makes sense.

> 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
>            Assignee: Dominique Jäggi
>            Priority: Critical
>             Fix For: 1.6, 1.4.8
>
>         Attachments: OAK-4845-test-lostmembership.patch, 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)