Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZEPPELIN-2161 Nested Group Support in LdapRealm for AD #2062

Closed
wants to merge 1 commit into from

Conversation

weand
Copy link
Contributor

@weand weand commented Feb 23, 2017

What is this PR for?

A common use case in LDAP/AD setup is the hierarchical structuring of
groups - a.k.a. adding groups to other groups. Such nesting groups can
help reduce the number of roles that need to be managed.

Current zeppelin realm implementations doesn't have support for looking
up memberships throughout nested group structures.

E.g. consider the following nested group scenario:

acme_employees
 \__department_a
     \__sub_department_x

User 'bob' is in Group 'sub_department_x'.
Notebook 'note1' has a Reader Role assignment for 'department_a' or
'acme_employees'.
Then access must be granted for 'bob' on 'note1'.

In AD enviroments this scenarios can be efficiently implemented using
the so called LDAP_MATCHING_RULE_IN_CHAIN operator
'1.2.840.113556.1.4.1941'.

This PR introduces a property 'groupSearchEnableMatchingRuleInChain' to
org.apache.zeppelin.realm.LdapRealm which defaults to false. If enabled,
all roles of potential nested group hierarchies will be resolved using
the LDAP_MATCHING_RULE_IN_CHAIN operator.

What type of PR is it?

Improvement

Todos

What is the Jira issue?

[ZEPPELIN-2161]

How should this be tested?

Set groupSearchEnableMatchingRuleInChain = true for the ldap realm.

Screenshots (if appropriate)

Questions:

  • Does the licenses files need update? n
  • Is there breaking changes for older versions? n
  • Does this needs documentation? y

@weand
Copy link
Contributor Author

weand commented Feb 23, 2017

any idea why the build has failed? I don't get the error.

@Leemoonsoo
Copy link
Member

Thanks @weand for improvmenet.
Have you turn on "Build Pushes" in travis-ci settings?

image

@@ -128,6 +128,8 @@
private static final String SUBJECT_USER_GROUPS = "subject.userGroups";
private static final String MEMBER_URL = "memberUrl";
private static final String POSIX_GROUP = "posixGroup";
private static final String MATCHING_RULE_IN_CHAIN_FORMAT =
"(&(objectClass=%s)(%s:1.2.840.113556.1.4.1941:=%s))";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment about this magic number would be helpful in future debug. Also if you can specify the doc URL would be helpful.
I understand its This rule is limited to filters that apply to the DN. This is a special "extended" match operator that walks the chain of ancestry in objects all the way to the root until it finds a match.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, comment added.

@prabhjyotsingh
Copy link
Contributor

@weand
Copy link
Contributor Author

weand commented Feb 27, 2017

@prabhjyotsingh There isn't any documentation for org.apache.zeppelin.realm.LdapRealm yet.

@Leemoonsoo Any special reason why LdapRealm wasn't documented in 0.7.0 yet?

Can anyone help me why the build is failing. I don't see, why these tests now fail, they have no obvious reference to changes of this PR:
NotebookTest.testPublicPrivateNewNote:1173 expected:<2C[CJ4Y6RZ]> but was:<2C[ADXZJ8N]>
NotebookTest.testAuthorizationRoles:759 expected: but was:

@Leemoonsoo
Copy link
Member

@weand So far, we have a section for LdapRealm in shiroauthentication.md.

NotebookTest.testPublicPrivateNewNote:1173 expected:<2C[CJ4Y6RZ]> but was:<2C[ADXZJ8N]>
NotebookTest.testAuthorizationRoles:759 expected: but was:

This problem has been fixed in current master branch. Could you rebase or merge master and see if test passes?

@weand
Copy link
Contributor Author

weand commented Mar 2, 2017

@Leemoonsoo Rebased and green now. Thanks for your help.
Would you mind merging that into branch-0.7 as well?

Regarding docu:
The LDAP Realm section in the docu shiroauthentication.md only mentions LdapGroupRealm implementation. This change is related to the undocumented LdapRealm, which was introduced in 0.7.0.

Should we document two LDAP Realms, or should we mark the old one deprecated?

Because of that, I would prefer an separate Issue for the missing piece of documentation.

@Leemoonsoo
Copy link
Member

Okay, LGTM.
Merge master and branch-0.7 if no further discussion.

Regarding documentation, if LdapGroupRealm and LdapRealm are superset/subset relation, we can mark one deprecated. If not, i think we should have document two LDAP Realms.

asfgit pushed a commit that referenced this pull request Mar 5, 2017
### What is this PR for?
A common use case in LDAP/AD setup is the hierarchical structuring of
groups - a.k.a. adding groups to other groups. Such nesting groups can
help reduce the number of roles that need to be managed.

Current zeppelin realm implementations doesn't have support for looking
up memberships throughout nested group structures.

E.g. consider the following nested group scenario:
```
acme_employees
 \__department_a
     \__sub_department_x
```
User 'bob' is in Group 'sub_department_x'.
Notebook 'note1' has a Reader Role assignment for 'department_a' or
'acme_employees'.
Then access must be granted for 'bob' on 'note1'.

In AD enviroments this scenarios can be efficiently implemented using
the so called LDAP_MATCHING_RULE_IN_CHAIN operator
'1.2.840.113556.1.4.1941'.

This PR introduces a property 'groupSearchEnableMatchingRuleInChain' to
org.apache.zeppelin.realm.LdapRealm which defaults to false. If enabled,
all roles of potential nested group hierarchies will be resolved using
the LDAP_MATCHING_RULE_IN_CHAIN operator.

### What type of PR is it?
Improvement

### Todos
-

### What is the Jira issue?
[ZEPPELIN-2161]

### How should this be tested?
Set groupSearchEnableMatchingRuleInChain = true for the ldap realm.

### Screenshots (if appropriate)

### Questions:
* Does the licenses files need update? n
* Is there breaking changes for older versions? n
* Does this needs documentation? y

Author: Andreas Weise <a.weise@avm.de>

Closes #2062 from weand/ZEPPELIN-2161 and squashes the following commits:

c08d015 [Andreas Weise] ZEPPELIN-2161 Nested group support in LdapRealm for AD

(cherry picked from commit b9fa42d)
Signed-off-by: Lee moon soo <moon@apache.org>
@asfgit asfgit closed this in b9fa42d Mar 5, 2017
prabhjyotsingh pushed a commit to prabhjyotsingh/zeppelin that referenced this pull request Mar 7, 2017
### What is this PR for?
A common use case in LDAP/AD setup is the hierarchical structuring of
groups - a.k.a. adding groups to other groups. Such nesting groups can
help reduce the number of roles that need to be managed.

Current zeppelin realm implementations doesn't have support for looking
up memberships throughout nested group structures.

E.g. consider the following nested group scenario:
```
acme_employees
 \__department_a
     \__sub_department_x
```
User 'bob' is in Group 'sub_department_x'.
Notebook 'note1' has a Reader Role assignment for 'department_a' or
'acme_employees'.
Then access must be granted for 'bob' on 'note1'.

In AD enviroments this scenarios can be efficiently implemented using
the so called LDAP_MATCHING_RULE_IN_CHAIN operator
'1.2.840.113556.1.4.1941'.

This PR introduces a property 'groupSearchEnableMatchingRuleInChain' to
org.apache.zeppelin.realm.LdapRealm which defaults to false. If enabled,
all roles of potential nested group hierarchies will be resolved using
the LDAP_MATCHING_RULE_IN_CHAIN operator.

### What type of PR is it?
Improvement

### Todos
-

### What is the Jira issue?
[ZEPPELIN-2161]

### How should this be tested?
Set groupSearchEnableMatchingRuleInChain = true for the ldap realm.

### Screenshots (if appropriate)

### Questions:
* Does the licenses files need update? n
* Is there breaking changes for older versions? n
* Does this needs documentation? y

Author: Andreas Weise <a.weise@avm.de>

Closes apache#2062 from weand/ZEPPELIN-2161 and squashes the following commits:

c08d015 [Andreas Weise] ZEPPELIN-2161 Nested group support in LdapRealm for AD

(cherry picked from commit b9fa42d)
Signed-off-by: Lee moon soo <moon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants