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-3312 Add option to convert username to lower case #2923

Closed
wants to merge 7 commits into from

Conversation

VipinRathor
Copy link
Contributor

What is this PR for?

This PR introduces a new configuration property to convert username to lower case. This is useful when the users (from external sources like AD/LDAP) are coming in with mixed-case names and Hadoop services (like Hive) can't authorize them correctly because Hadoop services recognize users only in lower case (like Linux).

Adding a new config option "zeppelin.username.force.lowercase" to handle such scenarios.

Behavior without this PR:
Access is denied to CaMel case user while running a Hive paragraph

Behavior with this PR:
User is allowed to run query when proposed configuration set to true.
By default, keeping zeppelin.username.force.lowercase=false to retain the current behavior.

What type of PR is it?

[Bug Fix]

Todos

  • - Unit test

What is the Jira issue?

How should this be tested?

  • Travis CI should pass
  • Manual steps to test:
  1. Configure Zeppelin with Active Directory authentication
  2. Login to Zeppelin as a CaMel case user
  3. Try to run a simple JDBC note with a Hive query (like a select * query). This would fail with "user [CaMel] does not have proper privileges to [USE] operation" error message.
  4. Now set zeppelin.username.force.lowercase=true in custom zeppelin-site.xml configuration.
  5. Once again, login as CaMel case user. This time the same Hive query would run as expected. Because the username is now passed in lower case.
  6. Also notice that after successful login, the login username (in the top-right corner) will be in lower case too.

Screenshots (if appropriate)

  • Login as CaMel case user:

screen shot 2018-04-12 at 3 33 01 am

* Notice the converted username post login:

screen shot 2018-04-12 at 3 33 43 am

Questions:

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

@VipinRathor
Copy link
Contributor Author

@prabhjyotsingh @felixcheung @zjffdu Can you please help review? Thanks.

@VipinRathor
Copy link
Contributor Author

This can also be tested via REST APIs like this:

  1. Login and get cookie as CaMel case user:
    curl -iv -b /tmp/cookie -c /tmp/cookie --data "userName=CaMel&password=passw0rd" -X POST http://localhost:9995/api/login

  2. Access a notebook or run some paragraph using the same cookie:
    curl -iv -b /tmp/cookie -c /tmp/cookie http://localhost:9995/api/notebook/2D9Q95A3K

  3. Operation will be allowed with following debug log message:
    DEBUG [2018-04-12 08:33:20,274] ({qtp2146608740-19 - /api/notebook/2D9Q95A3K} SecurityUtils.java[getPrincipal]:105) - converting principal name CaMel to lower case:camel

@Tagar
Copy link
Contributor

Tagar commented Apr 12, 2018

Great idea.. I had a similar jira https://issues.apache.org/jira/browse/ZEPPELIN-2886
Thanks!

@VipinRathor VipinRathor reopened this Apr 12, 2018
@VipinRathor
Copy link
Contributor Author

All Travis CI builds are failing for some npm error. Looks unrelated but I'm not sure. I need some help in getting this through.
CC: @prabhjyotsingh @r-kamath

@zjffdu
Copy link
Contributor

zjffdu commented Apr 13, 2018

LGTM @prabhjyotsingh could you help take a look at the npm error in travis ? Some other contributors also hit this same in other PR recently.

@r-kamath
Copy link
Member

@VipinRathor can you try removing npm cache clear from .travis.yml ? otherwise add --force.

npm doc says.

All data that passes through the cache is fully verified for integrity on both insertion and extraction. Cache corruption will either trigger an error, or signal to pacote that the data must be refetched, which it will do automatically. For this reason, it should never be necessary to clear the cache for any reason other than reclaiming disk space, thus why clean now requires --force to run.
details

@r-kamath
Copy link
Member

@VipinRathor #2928

<property>
<name>zeppelin.username.force.lowercase</name>
<value>false</value>
<description>Force convert username case to lower case, useful for Active Directory. Default is not to change case</description>
Copy link
Member

Choose a reason for hiding this comment

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

could you elaborate more on , useful for Active Directory? say as Active Directory/LDAP are case-insensitive?
also, isn't this applicable to LDAP too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this is very much applicable to LDAP. I'll update in the next commit.

@@ -91,6 +92,11 @@ public static String getPrincipal() {
String principal;
if (subject.isAuthenticated()) {
principal = extractPrincipal(subject);
if (ZeppelinServer.notebook.getConf().isUsernameForceLowerCase()) {
log.debug("Converting principal name " + principal
+ " to lower case:" + principal.toLowerCase());
Copy link
Member

Choose a reason for hiding this comment

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

log.warn()? since this is possibly security related.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can do but then there will be lot of noise at info log level. Are we sure that we want to have this?

@VipinRathor
Copy link
Contributor Author

5/11 builds are still failing but due to unrelated errors. Tried restarting failed one multiple times. All 5 are showing different errors. We can either ignore them or fix them in separate issues.

@VipinRathor
Copy link
Contributor Author

After few more retries, only 2/11 are failing now. Just restarted them again with the hope that those 2 will succeed too.

@Tagar
Copy link
Contributor

Tagar commented Apr 18, 2018

I was thinking we should also lower-case usernames that are stored in notebook-authorization.json, when doing authorization. Otherwise users wouldn't be able to see their own notebooks when this option is enabled and they used case inconsistently when were logging into Zeppelin before.

@VipinRathor
Copy link
Contributor Author

@Tagar, me & @prabhjyotsingh did discuss notebook authorization changes but somehow I missed to include them. I committed those changes yesterday. Now notebook authorization will also be mindful of username case conversion. Thanks for pointing out!

Today, we also discovered that similar to notebook authorization, interpreter "set permission" is another place where case conversion will be required. I've just committed those changes as well.

Copy link
Contributor

@prabhjyotsingh prabhjyotsingh left a comment

Choose a reason for hiding this comment

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

LGTM

@prabhjyotsingh
Copy link
Contributor

https://travis-ci.org/VipinRathor/zeppelin/builds/369033290 looks flaky, works well on local.

Merging this to master and branch-0.8 if no more discussion.

@asfgit asfgit closed this in b89c9ad Apr 23, 2018
asfgit pushed a commit that referenced this pull request Apr 23, 2018
This PR introduces a new configuration property to convert username to lower case. This is useful when the users (from external sources like AD/LDAP) are coming in with mixed-case names and Hadoop services (like Hive) can't authorize them correctly because Hadoop services recognize users only in lower case (like Linux).

Adding a new config option "zeppelin.username.force.lowercase" to handle such scenarios.

Behavior without this PR:
Access is denied to CaMel case user while running a Hive paragraph

Behavior with this PR:
User is allowed to run query when proposed configuration set to true.
By default, keeping zeppelin.username.force.lowercase=false to retain the current behavior.

[Bug Fix]

* [ ] - Unit test

* https://issues.apache.org/jira/browse/ZEPPELIN-3312

* Travis CI should pass
* Manual steps to test:
1. Configure Zeppelin with Active Directory authentication
2. Login to Zeppelin as a CaMel case user
3. Try to run a simple JDBC note with a Hive query (like a select * query). This would fail with "user [CaMel] does not have proper privileges to [USE] operation" error message.
4. Now set zeppelin.username.force.lowercase=true in custom zeppelin-site.xml configuration.
5. Once again, login as CaMel case user. This time the same Hive query would run as expected. Because the username is now passed in lower case.
6. Also notice that after successful login, the login username (in the top-right corner) will be in lower case too.

* Login as CaMel case user:
<img width="596" alt="screen shot 2018-04-12 at 3 33 01 am" src="https://user-images.githubusercontent.com/15668387/38672744-faf00f8e-3e03-11e8-86b2-cc5981d380d2.png">
* Notice the converted username post login:
<img width="806" alt="screen shot 2018-04-12 at 3 33 43 am" src="https://user-images.githubusercontent.com/15668387/38672777-108c7b66-3e04-11e8-8c97-467b4b73fe3d.png">

* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Vipin Rathor <v.rathor@gmail.com>

Closes #2923 from VipinRathor/ZEPPELIN-3312 and squashes the following commits:

886acb9 [Vipin Rathor] Add lowercase username support for interpreter permission
c112098 [Vipin Rathor] Remove maven cyclic refernce and use conf object directly
549f84d [Vipin Rathor] Convert username for Notebook Authorization as well
f83ce9f [Vipin Rathor] Adding new test testUsernameForceLowerCase and fixing canGetPrincipalName
b272299 [Vipin Rathor] Fixing Travis CI build failure due to indentation
83fb686 [Vipin Rathor] Incorporating PR review suggestion to zeppelin-site.xml-template
7f9b4df [Vipin Rathor] Add support to force username case conversion

Change-Id: Id13a5eca9718063cb629454a9d3d2fc6f3cfb663
(cherry picked from commit b89c9ad)
Signed-off-by: Prabhjyot Singh <prabhjyotsingh@gmail.com>

# Conflicts:
#	zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java
#	zeppelin-server/src/test/java/org/apache/zeppelin/security/SecurityUtilsTest.java
jwagun pushed a commit to jwagun/zeppelin that referenced this pull request Apr 23, 2018
This PR introduces a new configuration property to convert username to lower case. This is useful when the users (from external sources like AD/LDAP) are coming in with mixed-case names and Hadoop services (like Hive) can't authorize them correctly because Hadoop services recognize users only in lower case (like Linux).

Adding a new config option "zeppelin.username.force.lowercase" to handle such scenarios.

Behavior without this PR:
Access is denied to CaMel case user while running a Hive paragraph

Behavior with this PR:
User is allowed to run query when proposed configuration set to true.
By default, keeping zeppelin.username.force.lowercase=false to retain the current behavior.

[Bug Fix]

* [ ] - Unit test

* https://issues.apache.org/jira/browse/ZEPPELIN-3312

* Travis CI should pass
* Manual steps to test:
1. Configure Zeppelin with Active Directory authentication
2. Login to Zeppelin as a CaMel case user
3. Try to run a simple JDBC note with a Hive query (like a select * query). This would fail with "user [CaMel] does not have proper privileges to [USE] operation" error message.
4. Now set zeppelin.username.force.lowercase=true in custom zeppelin-site.xml configuration.
5. Once again, login as CaMel case user. This time the same Hive query would run as expected. Because the username is now passed in lower case.
6. Also notice that after successful login, the login username (in the top-right corner) will be in lower case too.

* Login as CaMel case user:
<img width="596" alt="screen shot 2018-04-12 at 3 33 01 am" src="https://user-images.githubusercontent.com/15668387/38672744-faf00f8e-3e03-11e8-86b2-cc5981d380d2.png">
* Notice the converted username post login:
<img width="806" alt="screen shot 2018-04-12 at 3 33 43 am" src="https://user-images.githubusercontent.com/15668387/38672777-108c7b66-3e04-11e8-8c97-467b4b73fe3d.png">

* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Vipin Rathor <v.rathor@gmail.com>

Closes apache#2923 from VipinRathor/ZEPPELIN-3312 and squashes the following commits:

886acb9 [Vipin Rathor] Add lowercase username support for interpreter permission
c112098 [Vipin Rathor] Remove maven cyclic refernce and use conf object directly
549f84d [Vipin Rathor] Convert username for Notebook Authorization as well
f83ce9f [Vipin Rathor] Adding new test testUsernameForceLowerCase and fixing canGetPrincipalName
b272299 [Vipin Rathor] Fixing Travis CI build failure due to indentation
83fb686 [Vipin Rathor] Incorporating PR review suggestion to zeppelin-site.xml-template
7f9b4df [Vipin Rathor] Add support to force username case conversion

Change-Id: Id13a5eca9718063cb629454a9d3d2fc6f3cfb663
prabhjyotsingh pushed a commit to prabhjyotsingh/zeppelin that referenced this pull request Jul 4, 2018
This PR introduces a new configuration property to convert username to lower case. This is useful when the users (from external sources like AD/LDAP) are coming in with mixed-case names and Hadoop services (like Hive) can't authorize them correctly because Hadoop services recognize users only in lower case (like Linux).

Adding a new config option "zeppelin.username.force.lowercase" to handle such scenarios.

Behavior without this PR:
Access is denied to CaMel case user while running a Hive paragraph

Behavior with this PR:
User is allowed to run query when proposed configuration set to true.
By default, keeping zeppelin.username.force.lowercase=false to retain the current behavior.

[Bug Fix]

* [ ] - Unit test

* https://issues.apache.org/jira/browse/ZEPPELIN-3312

* Travis CI should pass
* Manual steps to test:
1. Configure Zeppelin with Active Directory authentication
2. Login to Zeppelin as a CaMel case user
3. Try to run a simple JDBC note with a Hive query (like a select * query). This would fail with "user [CaMel] does not have proper privileges to [USE] operation" error message.
4. Now set zeppelin.username.force.lowercase=true in custom zeppelin-site.xml configuration.
5. Once again, login as CaMel case user. This time the same Hive query would run as expected. Because the username is now passed in lower case.
6. Also notice that after successful login, the login username (in the top-right corner) will be in lower case too.

* Login as CaMel case user:
<img width="596" alt="screen shot 2018-04-12 at 3 33 01 am" src="https://user-images.githubusercontent.com/15668387/38672744-faf00f8e-3e03-11e8-86b2-cc5981d380d2.png">
* Notice the converted username post login:
<img width="806" alt="screen shot 2018-04-12 at 3 33 43 am" src="https://user-images.githubusercontent.com/15668387/38672777-108c7b66-3e04-11e8-8c97-467b4b73fe3d.png">

* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Vipin Rathor <v.rathor@gmail.com>

Closes apache#2923 from VipinRathor/ZEPPELIN-3312 and squashes the following commits:

886acb9 [Vipin Rathor] Add lowercase username support for interpreter permission
c112098 [Vipin Rathor] Remove maven cyclic refernce and use conf object directly
549f84d [Vipin Rathor] Convert username for Notebook Authorization as well
f83ce9f [Vipin Rathor] Adding new test testUsernameForceLowerCase and fixing canGetPrincipalName
b272299 [Vipin Rathor] Fixing Travis CI build failure due to indentation
83fb686 [Vipin Rathor] Incorporating PR review suggestion to zeppelin-site.xml-template
7f9b4df [Vipin Rathor] Add support to force username case conversion

Change-Id: Id13a5eca9718063cb629454a9d3d2fc6f3cfb663
(cherry picked from commit b89c9ad)
Signed-off-by: Prabhjyot Singh <prabhjyotsingh@gmail.com>
mckartha pushed a commit to syntechdev/zeppelin that referenced this pull request Aug 9, 2018
This PR introduces a new configuration property to convert username to lower case. This is useful when the users (from external sources like AD/LDAP) are coming in with mixed-case names and Hadoop services (like Hive) can't authorize them correctly because Hadoop services recognize users only in lower case (like Linux).

Adding a new config option "zeppelin.username.force.lowercase" to handle such scenarios.

Behavior without this PR:
Access is denied to CaMel case user while running a Hive paragraph

Behavior with this PR:
User is allowed to run query when proposed configuration set to true.
By default, keeping zeppelin.username.force.lowercase=false to retain the current behavior.

[Bug Fix]

* [ ] - Unit test

* https://issues.apache.org/jira/browse/ZEPPELIN-3312

* Travis CI should pass
* Manual steps to test:
1. Configure Zeppelin with Active Directory authentication
2. Login to Zeppelin as a CaMel case user
3. Try to run a simple JDBC note with a Hive query (like a select * query). This would fail with "user [CaMel] does not have proper privileges to [USE] operation" error message.
4. Now set zeppelin.username.force.lowercase=true in custom zeppelin-site.xml configuration.
5. Once again, login as CaMel case user. This time the same Hive query would run as expected. Because the username is now passed in lower case.
6. Also notice that after successful login, the login username (in the top-right corner) will be in lower case too.

* Login as CaMel case user:
<img width="596" alt="screen shot 2018-04-12 at 3 33 01 am" src="https://user-images.githubusercontent.com/15668387/38672744-faf00f8e-3e03-11e8-86b2-cc5981d380d2.png">
* Notice the converted username post login:
<img width="806" alt="screen shot 2018-04-12 at 3 33 43 am" src="https://user-images.githubusercontent.com/15668387/38672777-108c7b66-3e04-11e8-8c97-467b4b73fe3d.png">

* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Vipin Rathor <v.rathor@gmail.com>

Closes apache#2923 from VipinRathor/ZEPPELIN-3312 and squashes the following commits:

886acb9 [Vipin Rathor] Add lowercase username support for interpreter permission
c112098 [Vipin Rathor] Remove maven cyclic refernce and use conf object directly
549f84d [Vipin Rathor] Convert username for Notebook Authorization as well
f83ce9f [Vipin Rathor] Adding new test testUsernameForceLowerCase and fixing canGetPrincipalName
b272299 [Vipin Rathor] Fixing Travis CI build failure due to indentation
83fb686 [Vipin Rathor] Incorporating PR review suggestion to zeppelin-site.xml-template
7f9b4df [Vipin Rathor] Add support to force username case conversion

Change-Id: Id13a5eca9718063cb629454a9d3d2fc6f3cfb663
mckartha pushed a commit to syntechdev/zeppelin that referenced this pull request Aug 9, 2018
This PR introduces a new configuration property to convert username to lower case. This is useful when the users (from external sources like AD/LDAP) are coming in with mixed-case names and Hadoop services (like Hive) can't authorize them correctly because Hadoop services recognize users only in lower case (like Linux).

Adding a new config option "zeppelin.username.force.lowercase" to handle such scenarios.

Behavior without this PR:
Access is denied to CaMel case user while running a Hive paragraph

Behavior with this PR:
User is allowed to run query when proposed configuration set to true.
By default, keeping zeppelin.username.force.lowercase=false to retain the current behavior.

[Bug Fix]

* [ ] - Unit test

* https://issues.apache.org/jira/browse/ZEPPELIN-3312

* Travis CI should pass
* Manual steps to test:
1. Configure Zeppelin with Active Directory authentication
2. Login to Zeppelin as a CaMel case user
3. Try to run a simple JDBC note with a Hive query (like a select * query). This would fail with "user [CaMel] does not have proper privileges to [USE] operation" error message.
4. Now set zeppelin.username.force.lowercase=true in custom zeppelin-site.xml configuration.
5. Once again, login as CaMel case user. This time the same Hive query would run as expected. Because the username is now passed in lower case.
6. Also notice that after successful login, the login username (in the top-right corner) will be in lower case too.

* Login as CaMel case user:
<img width="596" alt="screen shot 2018-04-12 at 3 33 01 am" src="https://user-images.githubusercontent.com/15668387/38672744-faf00f8e-3e03-11e8-86b2-cc5981d380d2.png">
* Notice the converted username post login:
<img width="806" alt="screen shot 2018-04-12 at 3 33 43 am" src="https://user-images.githubusercontent.com/15668387/38672777-108c7b66-3e04-11e8-8c97-467b4b73fe3d.png">

* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Vipin Rathor <v.rathor@gmail.com>

Closes apache#2923 from VipinRathor/ZEPPELIN-3312 and squashes the following commits:

886acb9 [Vipin Rathor] Add lowercase username support for interpreter permission
c112098 [Vipin Rathor] Remove maven cyclic refernce and use conf object directly
549f84d [Vipin Rathor] Convert username for Notebook Authorization as well
f83ce9f [Vipin Rathor] Adding new test testUsernameForceLowerCase and fixing canGetPrincipalName
b272299 [Vipin Rathor] Fixing Travis CI build failure due to indentation
83fb686 [Vipin Rathor] Incorporating PR review suggestion to zeppelin-site.xml-template
7f9b4df [Vipin Rathor] Add support to force username case conversion

Change-Id: Id13a5eca9718063cb629454a9d3d2fc6f3cfb663
(cherry picked from commit b89c9ad)
Signed-off-by: Prabhjyot Singh <prabhjyotsingh@gmail.com>

# Conflicts:
#	zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java
#	zeppelin-server/src/test/java/org/apache/zeppelin/security/SecurityUtilsTest.java
@Tagar
Copy link
Contributor

Tagar commented Aug 16, 2018

Can you guys please have a look at https://issues.apache.org/jira/browse/ZEPPELIN-3719 in case if this change caused that problem ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants