[jira] [Comment Edited] (OAK-4397) DefaultSyncContext.syncMembership may sync group of a foreign IDP

2017-11-03 Thread Julian Reschke (JIRA)

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

Julian Reschke edited comment on OAK-4397 at 11/3/17 1:48 PM:
--

trunk: [r1745336|http://svn.apache.org/r1745336]
1.4: [r1757697|http://svn.apache.org/r1757697]
1.2: [r1757698|http://svn.apache.org/r1757698]




was (Author: reschke):
trunk: [r1745336|http://svn.apache.org/r1745336]
1.4: [r1757697|http://svn.apache.org/r1757697]


> DefaultSyncContext.syncMembership may sync group of a foreign IDP
> -
>
> Key: OAK-4397
> URL: https://issues.apache.org/jira/browse/OAK-4397
> Project: Jackrabbit Oak
>  Issue Type: Bug
>  Components: auth-external
>Reporter: angela
>Assignee: angela
>Priority: Critical
>  Labels: security
> Fix For: 1.4.7, 1.5.3, 1.6.0
>
>
> With the following scenario the {{DefaultSyncContext.syncMembership}} may end 
> up synchronizing (i.e. updating) a group defined by an foreign IDP and even 
> add the user to be synchronized as a new member:
> - configuration with different IDPs
> - foreign IDP synchronizes a given external group 'groupA' => rep:externalID 
> points to foreign-IDP (e.g. rep:externalId = 'groupA;foreignIDP')
> - my-IDP contains a group with the same ID (but obviously with a different 
> rep:externalID) and user that has declared group membership pointing to 
> 'groupA' from my IDP
> if synchronizing my user first the groupA will be created with a 
> rep:externalId = 'groupA;myIDP'.
> however, if the group has been synced before by the foreignIDP the code fails 
> to verify that an existing group 'groupA' really belongs to the same IDP and 
> thus may end up synchronizing the group and updating it's members.
> IMHO that's a critical issue as it violates the IDP boundaries.
> the fix is pretty trivial as it only requires testing for the IDP of the 
> existing group as we do it in other places (even in the same method).



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Comment Edited] (OAK-4397) DefaultSyncContext.syncMembership may sync group of a foreign IDP

2017-11-03 Thread Julian Reschke (JIRA)

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

Julian Reschke edited comment on OAK-4397 at 11/3/17 1:42 PM:
--

trunk: [r1745336|http://svn.apache.org/r1745336]
1.4: [r1757697|http://svn.apache.org/r1757697]



was (Author: reschke):
trunk: [r1745336|http://svn.apache.org/r1745336]

> DefaultSyncContext.syncMembership may sync group of a foreign IDP
> -
>
> Key: OAK-4397
> URL: https://issues.apache.org/jira/browse/OAK-4397
> Project: Jackrabbit Oak
>  Issue Type: Bug
>  Components: auth-external
>Reporter: angela
>Assignee: angela
>Priority: Critical
>  Labels: security
> Fix For: 1.4.7, 1.5.3, 1.6.0
>
>
> With the following scenario the {{DefaultSyncContext.syncMembership}} may end 
> up synchronizing (i.e. updating) a group defined by an foreign IDP and even 
> add the user to be synchronized as a new member:
> - configuration with different IDPs
> - foreign IDP synchronizes a given external group 'groupA' => rep:externalID 
> points to foreign-IDP (e.g. rep:externalId = 'groupA;foreignIDP')
> - my-IDP contains a group with the same ID (but obviously with a different 
> rep:externalID) and user that has declared group membership pointing to 
> 'groupA' from my IDP
> if synchronizing my user first the groupA will be created with a 
> rep:externalId = 'groupA;myIDP'.
> however, if the group has been synced before by the foreignIDP the code fails 
> to verify that an existing group 'groupA' really belongs to the same IDP and 
> thus may end up synchronizing the group and updating it's members.
> IMHO that's a critical issue as it violates the IDP boundaries.
> the fix is pretty trivial as it only requires testing for the IDP of the 
> existing group as we do it in other places (even in the same method).



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Comment Edited] (OAK-4397) DefaultSyncContext.syncMembership may sync group of a foreign IDP

2016-10-07 Thread Alexander Klimetschek (JIRA)

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

Alexander Klimetschek edited comment on OAK-4397 at 10/7/16 7:44 PM:
-

Turns out, OAK-4845 is not a regression, as boundaries need to be established 
between multiple IDPs as well as possibly locally set memberships.

*Possible workarounds*

Existing applications relying on the previous behavior that allowed adding 
memberships to local groups (and these were really only intended as 
"placeholder" for the external groups) could do this:
* right before a batch sync is run, remove the local group and get it 
automatically recreated using the right {{rep:externalId}} (if temporarily 
loosing the group has no bad consequences; note that ACs would not be affected)
* temporarily disable the {{rep:externalId}} protection 
({{protectExternalId=false}}, see 
[docs|http://jackrabbit.apache.org/oak/docs/security/authentication/external/defaultusersync.html#Configuration_of_the_ExternalPrincipalConfiguration]),
 set the correct {{rep:externalId}} values on existing groups, enable 
protection again

Also, in the general principal and access control design, the external group 
might be used solely for the representation of the memberships. A separate 
local "role" group would be used for defining the access control, and this one 
would include the external group as member, so the permissions get inherited to 
the external users that are member of that group.

Please note that this only affects external groups, but not the 
{{user.autoMembership}} setting of the DefaultSyncHandler.


was (Author: alexander.klimetschek):
Turns out, OAK-4845 is not a regression, as boundaries need to be established 
between multiple IDPs as well as possibly locally set memberships.

Possible workarounds for existing applications relying on the previous behavior 
that allowed adding memberships to local groups (and these were really only 
intended as "placeholder" for the external groups):
* right before a batch sync is run, remove the local group and get it 
automatically recreated using the right {{rep:externalId}} (if temporarily 
loosing the group has no bad consequences; note that ACs would not be affected)
* temporarily disable the {{rep:externalId}} protection 
({{protectExternalId=false}}, see 
[docs|http://jackrabbit.apache.org/oak/docs/security/authentication/external/defaultusersync.html#Configuration_of_the_ExternalPrincipalConfiguration]),
 set the correct {{rep:externalId}} values on existing groups, enable 
protection again

Also, in the general principal and access control design, the external group 
might be used solely for the representation of the memberships. A separate 
"role" group would be used for defining the access control, and this one would 
include the external group as member, so the permissions get inherited to the 
external users that are member of that group.

Please note that this only affects external groups, but not the 
{{user.autoMembership}} setting of the DefaultSyncHandler.

> DefaultSyncContext.syncMembership may sync group of a foreign IDP
> -
>
> Key: OAK-4397
> URL: https://issues.apache.org/jira/browse/OAK-4397
> Project: Jackrabbit Oak
>  Issue Type: Bug
>  Components: auth-external
>Reporter: angela
>Assignee: angela
>Priority: Critical
>  Labels: security
> Fix For: 1.5.3, 1.4.7
>
>
> With the following scenario the {{DefaultSyncContext.syncMembership}} may end 
> up synchronizing (i.e. updating) a group defined by an foreign IDP and even 
> add the user to be synchronized as a new member:
> - configuration with different IDPs
> - foreign IDP synchronizes a given external group 'groupA' => rep:externalID 
> points to foreign-IDP (e.g. rep:externalId = 'groupA;foreignIDP')
> - my-IDP contains a group with the same ID (but obviously with a different 
> rep:externalID) and user that has declared group membership pointing to 
> 'groupA' from my IDP
> if synchronizing my user first the groupA will be created with a 
> rep:externalId = 'groupA;myIDP'.
> however, if the group has been synced before by the foreignIDP the code fails 
> to verify that an existing group 'groupA' really belongs to the same IDP and 
> thus may end up synchronizing the group and updating it's members.
> IMHO that's a critical issue as it violates the IDP boundaries.
> the fix is pretty trivial as it only requires testing for the IDP of the 
> existing group as we do it in other places (even in the same method).



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


[jira] [Comment Edited] (OAK-4397) DefaultSyncContext.syncMembership may sync group of a foreign IDP

2016-10-07 Thread Alexander Klimetschek (JIRA)

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

Alexander Klimetschek edited comment on OAK-4397 at 10/7/16 7:42 PM:
-

Turns out, OAK-4845 is not a regression, as boundaries need to be established 
between multiple IDPs as well as possibly locally set memberships.

Possible workarounds for existing applications relying on the previous behavior 
that allowed adding memberships to local groups (and these were really only 
intended as "placeholder" for the external groups):
* right before a batch sync is run, remove the local group and get it 
automatically recreated using the right {{rep:externalId}} (if temporarily 
loosing the group has no bad consequences; note that ACs would not be affected)
* temporarily disable the {{rep:externalId}} protection 
({{protectExternalId=false}}, see 
[docs|http://jackrabbit.apache.org/oak/docs/security/authentication/external/defaultusersync.html#Configuration_of_the_ExternalPrincipalConfiguration]),
 set the correct {{rep:externalId}} values on existing groups, enable 
protection again

Also, in the general principal and access control design, the external group 
might be used solely for the representation of the memberships. A separate 
"role" group would be used for defining the access control, and this one would 
include the external group as member, so the permissions get inherited to the 
external users that are member of that group.

Please note that this only affects external groups, but not the 
{{user.autoMembership}} setting of the DefaultSyncHandler.


was (Author: alexander.klimetschek):
Turns out, OAK-4845 is not a regression, as boundaries need to be established 
between multiple IDPs as well as possibly locally set memberships.

Possible workarounds for existing applications relying on the previous behavior 
that allowed adding memberships to local groups (and these were really only 
intended as "placeholder" for the external groups):
* right before a batch sync is run, remove the local group and get it 
automatically recreated using the right {{rep:externalId}} (if temporarily 
loosing the group has no bad consequences; note that ACs would not be affected)
* temporarily disable the {{rep:externalId}} protection 
({{protectExternalId=false}}, see 
[docs|http://jackrabbit.apache.org/oak/docs/security/authentication/external/defaultusersync.html#Configuration_of_the_ExternalPrincipalConfiguration]),
 set the correct {{rep:externalId}} values on existing groups, enable 
protection again

> DefaultSyncContext.syncMembership may sync group of a foreign IDP
> -
>
> Key: OAK-4397
> URL: https://issues.apache.org/jira/browse/OAK-4397
> Project: Jackrabbit Oak
>  Issue Type: Bug
>  Components: auth-external
>Reporter: angela
>Assignee: angela
>Priority: Critical
>  Labels: security
> Fix For: 1.5.3, 1.4.7
>
>
> With the following scenario the {{DefaultSyncContext.syncMembership}} may end 
> up synchronizing (i.e. updating) a group defined by an foreign IDP and even 
> add the user to be synchronized as a new member:
> - configuration with different IDPs
> - foreign IDP synchronizes a given external group 'groupA' => rep:externalID 
> points to foreign-IDP (e.g. rep:externalId = 'groupA;foreignIDP')
> - my-IDP contains a group with the same ID (but obviously with a different 
> rep:externalID) and user that has declared group membership pointing to 
> 'groupA' from my IDP
> if synchronizing my user first the groupA will be created with a 
> rep:externalId = 'groupA;myIDP'.
> however, if the group has been synced before by the foreignIDP the code fails 
> to verify that an existing group 'groupA' really belongs to the same IDP and 
> thus may end up synchronizing the group and updating it's members.
> IMHO that's a critical issue as it violates the IDP boundaries.
> the fix is pretty trivial as it only requires testing for the IDP of the 
> existing group as we do it in other places (even in the same method).



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


[jira] [Comment Edited] (OAK-4397) DefaultSyncContext.syncMembership may sync group of a foreign IDP

2016-10-07 Thread Alexander Klimetschek (JIRA)

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

Alexander Klimetschek edited comment on OAK-4397 at 10/7/16 7:39 PM:
-

Turns out, OAK-4845 is not a regression, as boundaries need to be established 
between multiple IDPs as well as possibly locally set memberships.

Possible workarounds for existing applications relying on the previous behavior 
that allowed adding memberships to local groups (and these were really only 
intended as "placeholder" for the external groups):
* right before a batch sync is run, remove the local group and get it 
automatically recreated using the right {{rep:externalId}} (if temporarily 
loosing the group has no bad consequences; note that ACs would not be affected)
* temporarily disable the {{rep:externalId}} protection 
({{protectExternalId=false}}, see 
[docs|http://jackrabbit.apache.org/oak/docs/security/authentication/external/defaultusersync.html#Configuration_of_the_ExternalPrincipalConfiguration]),
 set the correct {{rep:externalId}} values on existing groups, enable 
protection again


was (Author: alexander.klimetschek):
Turns out, OAK-4845 is not a regression, as it would only get more complex down 
the line.

Possible workarounds for existing applications relying on the previous behavior 
that allowed adding memberships to local groups (and these were really only 
intended as "placeholder" for the external groups):
* right before a batch sync is run, remove the local group and get it 
automatically recreated using the right {{rep:externalId}} (if temporarily 
loosing the group has no bad consequences; note that ACs would not be affected)
* temporarily disable the {{rep:externalId}} protection 
({{protectExternalId=false}}, see 
[docs|http://jackrabbit.apache.org/oak/docs/security/authentication/external/defaultusersync.html#Configuration_of_the_ExternalPrincipalConfiguration]),
 set the correct {{rep:externalId}} values on existing groups, enable 
protection again

> DefaultSyncContext.syncMembership may sync group of a foreign IDP
> -
>
> Key: OAK-4397
> URL: https://issues.apache.org/jira/browse/OAK-4397
> Project: Jackrabbit Oak
>  Issue Type: Bug
>  Components: auth-external
>Reporter: angela
>Assignee: angela
>Priority: Critical
>  Labels: security
> Fix For: 1.5.3, 1.4.7
>
>
> With the following scenario the {{DefaultSyncContext.syncMembership}} may end 
> up synchronizing (i.e. updating) a group defined by an foreign IDP and even 
> add the user to be synchronized as a new member:
> - configuration with different IDPs
> - foreign IDP synchronizes a given external group 'groupA' => rep:externalID 
> points to foreign-IDP (e.g. rep:externalId = 'groupA;foreignIDP')
> - my-IDP contains a group with the same ID (but obviously with a different 
> rep:externalID) and user that has declared group membership pointing to 
> 'groupA' from my IDP
> if synchronizing my user first the groupA will be created with a 
> rep:externalId = 'groupA;myIDP'.
> however, if the group has been synced before by the foreignIDP the code fails 
> to verify that an existing group 'groupA' really belongs to the same IDP and 
> thus may end up synchronizing the group and updating it's members.
> IMHO that's a critical issue as it violates the IDP boundaries.
> the fix is pretty trivial as it only requires testing for the IDP of the 
> existing group as we do it in other places (even in the same method).



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


[jira] [Comment Edited] (OAK-4397) DefaultSyncContext.syncMembership may sync group of a foreign IDP

2016-10-07 Thread Alexander Klimetschek (JIRA)

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

Alexander Klimetschek edited comment on OAK-4397 at 10/7/16 7:25 PM:
-

Turns out, OAK-4845 is not a regression, as it would only get more complex down 
the line.

Possible workarounds for existing applications relying on the previous behavior 
that allowed adding memberships to local groups (and these were really only 
intended as "placeholder" for the external groups):
* right before a batch sync is run, remove the local group and get it 
automatically recreated using the right {{rep:externalId}} (if temporarily 
loosing the group has no bad consequences; note that ACs would not be affected)
* temporarily disable the {{rep:externalId}} protection 
({{protectExternalId=false}}, see 
[docs|http://jackrabbit.apache.org/oak/docs/security/authentication/external/defaultusersync.html#Configuration_of_the_ExternalPrincipalConfiguration]),
 set the correct {{rep:externalId}} values on existing groups, enable 
protection again


was (Author: alexander.klimetschek):
Turns out, OAK-4845 is not a regression, as it would only get more complex down 
the line.

Removing membership from local groups could create problems, if both a local 
and external group accidentally have the same name. A completely separate, 
locally established group membership should not be inadvertently be removed by 
a sync. Furthermore, supporting adding memberships to local groups on one hand, 
but not removing, creates an inconsistency and would not provide the guarantee 
expected from the sync, that the group memberships in the JCR represent the 
ones from the external IDP.

Possible workarounds for existing applications relying on the previous behavior 
that allowed adding memberships, and had local groups that really are only used 
as placeholder for the external groups:
* right before a batch sync is run, remove the local group and get it 
automatically recreated using the right {{rep:externalId}} (if temporarily 
loosing the group has no bad consequences; note that ACs would not be affected)
* temporarily disable the {{rep:externalId}} protection 
({{protectExternalId=false}}, see 
[docs|http://jackrabbit.apache.org/oak/docs/security/authentication/external/defaultusersync.html#Configuration_of_the_ExternalPrincipalConfiguration]),
 set the correct {{rep:externalId}} values on existing groups, enable 
protection again

> DefaultSyncContext.syncMembership may sync group of a foreign IDP
> -
>
> Key: OAK-4397
> URL: https://issues.apache.org/jira/browse/OAK-4397
> Project: Jackrabbit Oak
>  Issue Type: Bug
>  Components: auth-external
>Reporter: angela
>Assignee: angela
>Priority: Critical
>  Labels: security
> Fix For: 1.5.3, 1.4.7
>
>
> With the following scenario the {{DefaultSyncContext.syncMembership}} may end 
> up synchronizing (i.e. updating) a group defined by an foreign IDP and even 
> add the user to be synchronized as a new member:
> - configuration with different IDPs
> - foreign IDP synchronizes a given external group 'groupA' => rep:externalID 
> points to foreign-IDP (e.g. rep:externalId = 'groupA;foreignIDP')
> - my-IDP contains a group with the same ID (but obviously with a different 
> rep:externalID) and user that has declared group membership pointing to 
> 'groupA' from my IDP
> if synchronizing my user first the groupA will be created with a 
> rep:externalId = 'groupA;myIDP'.
> however, if the group has been synced before by the foreignIDP the code fails 
> to verify that an existing group 'groupA' really belongs to the same IDP and 
> thus may end up synchronizing the group and updating it's members.
> IMHO that's a critical issue as it violates the IDP boundaries.
> the fix is pretty trivial as it only requires testing for the IDP of the 
> existing group as we do it in other places (even in the same method).



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


[jira] [Comment Edited] (OAK-4397) DefaultSyncContext.syncMembership may sync group of a foreign IDP

2016-09-22 Thread Alexander Klimetschek (JIRA)

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

Alexander Klimetschek edited comment on OAK-4397 at 9/23/16 12:11 AM:
--

This created a regression, in that it no longer allows to add memberships to 
locally defined groups (i.e. exist locally already, but no other IDP). Created 
OAK-4845

BTW, I noticed this has been [backported to the 1.4 
branch|https://github.com/apache/jackrabbit-oak/commit/d0251e3efbd6f24e47c06a0d7394816748ef18f8]
 already, the fix version might need an update.


was (Author: alexander.klimetschek):
_Update: created OAK-4845_

This introduced a regression: we have a group X created locally (used for 
setting up ACs in application packages etc.), and our custom external identity 
provider maps to it as an ExternalGroup. However, when running a new 
oak-auth-external version with this change, we get "Existing authorizable 'X' 
is not a group from this IDP 'foo'.", and the group is not synced and most 
importantly, memberships are not updated anymore. Looking at the group in JCR, 
it does not have a {{rep:externalId}} at all, only a {{rep:lastSynced}}.

AFAICS, this is because the {{rep:externalId}} is only ever set in 
{{createGroup()}}, i.e. when the external sync creates that group initially. If 
it's already there (but not owned by another IDP!), then it won't use it.

Note that we don't care for the properties of the group, we just need it to add 
users to it based on some rules on the external identity system side (it's a 
bit more complex system on that side). So maybe the new check should only apply 
to syncProperties for the group, but not for adding users to the group.

BTW, I noticed this has been [backported to the 1.4 
branch|https://github.com/apache/jackrabbit-oak/commit/d0251e3efbd6f24e47c06a0d7394816748ef18f8]
 already, the fix version might need an update.

> DefaultSyncContext.syncMembership may sync group of a foreign IDP
> -
>
> Key: OAK-4397
> URL: https://issues.apache.org/jira/browse/OAK-4397
> Project: Jackrabbit Oak
>  Issue Type: Bug
>  Components: auth-external
>Reporter: angela
>Assignee: angela
>Priority: Critical
>  Labels: security
> Fix For: 1.5.3
>
>
> With the following scenario the {{DefaultSyncContext.syncMembership}} may end 
> up synchronizing (i.e. updating) a group defined by an foreign IDP and even 
> add the user to be synchronized as a new member:
> - configuration with different IDPs
> - foreign IDP synchronizes a given external group 'groupA' => rep:externalID 
> points to foreign-IDP (e.g. rep:externalId = 'groupA;foreignIDP')
> - my-IDP contains a group with the same ID (but obviously with a different 
> rep:externalID) and user that has declared group membership pointing to 
> 'groupA' from my IDP
> if synchronizing my user first the groupA will be created with a 
> rep:externalId = 'groupA;myIDP'.
> however, if the group has been synced before by the foreignIDP the code fails 
> to verify that an existing group 'groupA' really belongs to the same IDP and 
> thus may end up synchronizing the group and updating it's members.
> IMHO that's a critical issue as it violates the IDP boundaries.
> the fix is pretty trivial as it only requires testing for the IDP of the 
> existing group as we do it in other places (even in the same method).



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


[jira] [Comment Edited] (OAK-4397) DefaultSyncContext.syncMembership may sync group of a foreign IDP

2016-09-22 Thread Alexander Klimetschek (JIRA)

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

Alexander Klimetschek edited comment on OAK-4397 at 9/22/16 10:41 PM:
--

_Update: created OAK-4845_

This introduced a regression: we have a group X created locally (used for 
setting up ACs in application packages etc.), and our custom external identity 
provider maps to it as an ExternalGroup. However, when running a new 
oak-auth-external version with this change, we get "Existing authorizable 'X' 
is not a group from this IDP 'foo'.", and the group is not synced and most 
importantly, memberships are not updated anymore. Looking at the group in JCR, 
it does not have a {{rep:externalId}} at all, only a {{rep:lastSynced}}.

AFAICS, this is because the {{rep:externalId}} is only ever set in 
{{createGroup()}}, i.e. when the external sync creates that group initially. If 
it's already there (but not owned by another IDP!), then it won't use it.

Note that we don't care for the properties of the group, we just need it to add 
users to it based on some rules on the external identity system side (it's a 
bit more complex system on that side). So maybe the new check should only apply 
to syncProperties for the group, but not for adding users to the group.

BTW, I noticed this has been [backported to the 1.4 
branch|https://github.com/apache/jackrabbit-oak/commit/d0251e3efbd6f24e47c06a0d7394816748ef18f8]
 already, the fix version might need an update.


was (Author: alexander.klimetschek):
This introduced a regression: we have a group X created locally (used for 
setting up ACs in application packages etc.), and our custom external identity 
provider maps to it as an ExternalGroup. However, when running a new 
oak-auth-external version with this change, we get "Existing authorizable 'X' 
is not a group from this IDP 'foo'.", and the group is not synced and most 
importantly, memberships are not updated anymore. Looking at the group in JCR, 
it does not have a {{rep:externalId}} at all, only a {{rep:lastSynced}}.

AFAICS, this is because the {{rep:externalId}} is only ever set in 
{{createGroup()}}, i.e. when the external sync creates that group initially. If 
it's already there (but not owned by another IDP!), then it won't use it.

Note that we don't care for the properties of the group, we just need it to add 
users to it based on some rules on the external identity system side (it's a 
bit more complex system on that side). So maybe the new check should only apply 
to syncProperties for the group, but not for adding users to the group.

BTW, I noticed this has been [backported to the 1.4 
branch|https://github.com/apache/jackrabbit-oak/commit/d0251e3efbd6f24e47c06a0d7394816748ef18f8]
 already, the fix version might need an update.

> DefaultSyncContext.syncMembership may sync group of a foreign IDP
> -
>
> Key: OAK-4397
> URL: https://issues.apache.org/jira/browse/OAK-4397
> Project: Jackrabbit Oak
>  Issue Type: Bug
>  Components: auth-external
>Reporter: angela
>Assignee: angela
>Priority: Critical
>  Labels: security
> Fix For: 1.5.3
>
>
> With the following scenario the {{DefaultSyncContext.syncMembership}} may end 
> up synchronizing (i.e. updating) a group defined by an foreign IDP and even 
> add the user to be synchronized as a new member:
> - configuration with different IDPs
> - foreign IDP synchronizes a given external group 'groupA' => rep:externalID 
> points to foreign-IDP (e.g. rep:externalId = 'groupA;foreignIDP')
> - my-IDP contains a group with the same ID (but obviously with a different 
> rep:externalID) and user that has declared group membership pointing to 
> 'groupA' from my IDP
> if synchronizing my user first the groupA will be created with a 
> rep:externalId = 'groupA;myIDP'.
> however, if the group has been synced before by the foreignIDP the code fails 
> to verify that an existing group 'groupA' really belongs to the same IDP and 
> thus may end up synchronizing the group and updating it's members.
> IMHO that's a critical issue as it violates the IDP boundaries.
> the fix is pretty trivial as it only requires testing for the IDP of the 
> existing group as we do it in other places (even in the same method).



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


[jira] [Comment Edited] (OAK-4397) DefaultSyncContext.syncMembership may sync group of a foreign IDP

2016-09-22 Thread Alexander Klimetschek (JIRA)

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

Alexander Klimetschek edited comment on OAK-4397 at 9/22/16 10:29 PM:
--

This introduced a regression: we have a group X created locally (used for 
setting up ACs in application packages etc.), and our custom external identity 
provider maps to it as an ExternalGroup. However, when running a new 
oak-auth-external version with this change, we get "Existing authorizable 'X' 
is not a group from this IDP 'foo'.", and the group is not synced and most 
importantly, memberships are not updated anymore. Looking at the group in JCR, 
it does not have a {{rep:externalId}} at all, only a {{rep:lastSynced}}.

AFAICS, this is because the {{rep:externalId}} is only ever set in 
{{createGroup()}}, i.e. when the external sync creates that group initially. If 
it's already there (but not owned by another IDP!), then it won't use it.

Note that we don't care for the properties of the group, we just need it to add 
users to it based on some rules on the external identity system side (it's a 
bit more complex system on that side). So maybe the new check should only apply 
to syncProperties for the group, but not for adding users to the group.

BTW, I noticed this has been [backported to the 1.4 
branch|https://github.com/apache/jackrabbit-oak/commit/d0251e3efbd6f24e47c06a0d7394816748ef18f8]
 already, the fix version might need an update.


was (Author: alexander.klimetschek):
This introduced a regression: we have a group X created locally (used for 
setting up ACs in application packages etc.), and our custom external identity 
provider maps to it as an ExternalGroup. However, when running a new 
oak-auth-external version with this change, we get "Existing authorizable 'X' 
is not a group from this IDP 'foo'.", and the group is not synced and most 
importantly, memberships are not updated anymore. Looking at the group in JCR, 
it does not have a {{rep:externalId}} at all, only a {{rep:lastSynced}}.

AFAICS, this is because the {{rep:externalId}} is only ever set in 
{{createGroup()}}, i.e. when the external sync creates that group initially. If 
it's already there (but not owned by another IDP!), then it can't use it.

Note that we don't care for the properties of the group, we just need it to add 
users to it based on some rules on the external identity system side (it's a 
bit more complex system on that side).

BTW, I noticed this has been [backported to the 1.4 
branch|https://github.com/apache/jackrabbit-oak/commit/d0251e3efbd6f24e47c06a0d7394816748ef18f8]
 already, the fix version might need an update.

> DefaultSyncContext.syncMembership may sync group of a foreign IDP
> -
>
> Key: OAK-4397
> URL: https://issues.apache.org/jira/browse/OAK-4397
> Project: Jackrabbit Oak
>  Issue Type: Bug
>  Components: auth-external
>Reporter: angela
>Assignee: angela
>Priority: Critical
>  Labels: security
> Fix For: 1.5.3
>
>
> With the following scenario the {{DefaultSyncContext.syncMembership}} may end 
> up synchronizing (i.e. updating) a group defined by an foreign IDP and even 
> add the user to be synchronized as a new member:
> - configuration with different IDPs
> - foreign IDP synchronizes a given external group 'groupA' => rep:externalID 
> points to foreign-IDP (e.g. rep:externalId = 'groupA;foreignIDP')
> - my-IDP contains a group with the same ID (but obviously with a different 
> rep:externalID) and user that has declared group membership pointing to 
> 'groupA' from my IDP
> if synchronizing my user first the groupA will be created with a 
> rep:externalId = 'groupA;myIDP'.
> however, if the group has been synced before by the foreignIDP the code fails 
> to verify that an existing group 'groupA' really belongs to the same IDP and 
> thus may end up synchronizing the group and updating it's members.
> IMHO that's a critical issue as it violates the IDP boundaries.
> the fix is pretty trivial as it only requires testing for the IDP of the 
> existing group as we do it in other places (even in the same method).



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


[jira] [Comment Edited] (OAK-4397) DefaultSyncContext.syncMembership may sync group of a foreign IDP

2016-09-22 Thread Alexander Klimetschek (JIRA)

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

Alexander Klimetschek edited comment on OAK-4397 at 9/22/16 10:26 PM:
--

This introduced a regression: we have a group X created locally (used for 
setting up ACs in application packages etc.), and our custom external identity 
provider maps to it as an ExternalGroup. However, when running a new 
oak-auth-external version with this change, we get "Existing authorizable 'X' 
is not a group from this IDP 'foo'.", and the group is not synced and most 
importantly, memberships are not updated anymore. Looking at the group in JCR, 
it does not have a {{rep:externalId}} at all, only a {{rep:lastSynced}}.

AFAICS, this is because the {{rep:externalId}} is only ever set in 
{{createGroup()}}, i.e. when the external sync creates that group initially. If 
it's already there (but not owned by another IDP!), then it can't use it.

Note that we don't care for the properties of the group, we just need it to add 
users to it based on some rules on the external identity system side (it's a 
bit more complex system on that side).

BTW, I noticed this has been [backported to the 1.4 
branch|https://github.com/apache/jackrabbit-oak/commit/d0251e3efbd6f24e47c06a0d7394816748ef18f8]
 already, the fix version might need an update.


was (Author: alexander.klimetschek):
This introduced a regression: we have a group X created locally (used for 
setting up ACs in application packages etc.), and our custom external identity 
provider maps to it as an ExternalGroup. However, when running a new 
oak-auth-external version with this change, we get "Existing authorizable 'X' 
is not a group from this IDP 'foo'.", and the group is not synced and most 
importantly, memberships are not updated anymore. Looking at the group in JCR, 
it does not have a {{rep:externalId}} at all, only a {{rep:lastSynced}}.

BTW, I noticed this has been [backported to the 1.4 
branch|https://github.com/apache/jackrabbit-oak/commit/d0251e3efbd6f24e47c06a0d7394816748ef18f8]
 already, the fix version might need an update.

> DefaultSyncContext.syncMembership may sync group of a foreign IDP
> -
>
> Key: OAK-4397
> URL: https://issues.apache.org/jira/browse/OAK-4397
> Project: Jackrabbit Oak
>  Issue Type: Bug
>  Components: auth-external
>Reporter: angela
>Assignee: angela
>Priority: Critical
>  Labels: security
> Fix For: 1.5.3
>
>
> With the following scenario the {{DefaultSyncContext.syncMembership}} may end 
> up synchronizing (i.e. updating) a group defined by an foreign IDP and even 
> add the user to be synchronized as a new member:
> - configuration with different IDPs
> - foreign IDP synchronizes a given external group 'groupA' => rep:externalID 
> points to foreign-IDP (e.g. rep:externalId = 'groupA;foreignIDP')
> - my-IDP contains a group with the same ID (but obviously with a different 
> rep:externalID) and user that has declared group membership pointing to 
> 'groupA' from my IDP
> if synchronizing my user first the groupA will be created with a 
> rep:externalId = 'groupA;myIDP'.
> however, if the group has been synced before by the foreignIDP the code fails 
> to verify that an existing group 'groupA' really belongs to the same IDP and 
> thus may end up synchronizing the group and updating it's members.
> IMHO that's a critical issue as it violates the IDP boundaries.
> the fix is pretty trivial as it only requires testing for the IDP of the 
> existing group as we do it in other places (even in the same method).



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