You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by hkropp <gi...@git.apache.org> on 2016/11/03 09:57:40 UTC
[GitHub] zeppelin pull request #1589: [Zeppelin-1611] - Support PAM (System User) Aut...
GitHub user hkropp opened a pull request:
https://github.com/apache/zeppelin/pull/1589
[Zeppelin-1611] - Support PAM (System User) Authentication
### What is this PR for?
This PR adds [PAM](https://en.wikipedia.org/wiki/Pluggable_authentication_module) authentication support based on the introduced Shiro security implementation. With PAM support system users have immediate access to a secured Zeppelin instance.
### What type of PR is it?
Feature
### Todos
* [x] - Create PAM realm
* [x] - Create test for PAM authentication
* [x] - Test with running Zeppelin instance
### What is the Jira issue?
[ZEPPELIN-1611](https://issues.apache.org/jira/browse/ZEPPELIN-1611])
### How should this be tested?
`PamRealmTest` executes an automated test if the environment variables `PAM_USER` and `PAM_PASS` are set. This should be set to system username and password.
The test also includes a main function to manually execute the test. Setting the environment variables for example on MacOS for your IDE use `launchctl setenv PAM_USER user` and `launchctl setenv PAM_PASS xxxxx`, the test can then be run from your IDE.
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? Yes
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hkropp/incubator-zeppelin ZEPPELIN-1611
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1589.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1589
----
commit 257f14e333c28c1b4b8f37e47ba9963221287c5c
Author: hkropp <hk...@hortonworks.com>
Date: 2016-11-03T09:35:36Z
ZEPPELIN-1611
commit b673c76be855d7a13f7b34fda0032c2f8040694c
Author: hkropp <hk...@hortonworks.com>
Date: 2016-11-03T09:35:45Z
Merge branch 'master' of github.com:apache/incubator-zeppelin into ZEPPELIN-1611
commit efa79afa47147d6e1caa0767d4929e3c752c64e3
Author: hkropp <hk...@hortonworks.com>
Date: 2016-11-03T09:35:36Z
ZEPPELIN-1611
commit 00cc0320840a08a76925dbfbf0494f0623c0e558
Author: Anthony Corbacho <co...@gmail.com>
Date: 2016-11-03T02:59:07Z
[ZEPPELIN-1586] Add security check in NotebookRestApi
### What is this PR for?
Bring some security check in `NotebookRestApi`.
### What type of PR is it?
[Bug Fix | Improvement | Refactoring]
### Todos
- [x] - Create a proper way to throw webapp error
- [x] - Add in `NotebookAuthorization` some method to check if user is owner, reader or writer
- [x] - Add Authorization check in `NotebookRestapi`
- [x] - Add New test for security in notebook rest api
### What is the Jira issue?
- [ZEPPELIN-1586](https://issues.apache.org/jira/browse/ZEPPELIN-1586)
### How should this be tested?
First, force Zeppelin to use auth.
- In `conf/zeppelin-site.xml` change `zeppelin.anonymous.allowed` to **false**
```
<property>
<name>zeppelin.anonymous.allowed</name>
<value>false</value>
<description>Anonymous user allowed by default</description>
</property>
```
- In `conf/shiro.ini` set Shiro to use `Auth` at the end of the file
```
#/** = anon
/** = authc
```
- Start Zeppelin, login and set some permission to a note
- try to get a note from Zeppelin Rest Api `http://localhost:8080/api/notebook/{noteId}` (you can use your browser or curl (if you use curl please add shiro token to curl cookie))
### Screenshots (if appropriate)
### Questions:
- Does the licenses files need update? No
- Is there breaking changes for older versions? No
- Does this needs documentation? Maybe
Author: Anthony Corbacho <co...@gmail.com>
Closes #1567 from anthonycorbacho/fix/ZEPPELIN-1586 and squashes the following commits:
6615935 [Anthony Corbacho] Clean anonymous allowed property when shutting down zeppelin server
30815c1 [Anthony Corbacho] Fix typo
bab7e60 [Anthony Corbacho] Rewording
decd1e9 [Anthony Corbacho] Simple implementation of notebook test with shiro (security)
b412266 [Anthony Corbacho] Refactored Abstract rest api test to also handle the case of tests with shiro (security), I also added some utility http method to do action with authenticated user
db0c39c [Anthony Corbacho] Adress review and fix typos
eacfa8e [Anthony Corbacho] Fix typo and bad copy paste for isOwner
c8c42b2 [Anthony Corbacho] Change cxf version from 2.7.7 to 2.7.8 to avoid method not found where throw WebAppException
ed404a4 [Anthony Corbacho] Rename permission check note :: be more meaningful
6030776 [Anthony Corbacho] Handle security check
fe380ab [Anthony Corbacho] Add webapp exception handler :)
21f9288 [Anthony Corbacho] Replace check of aninonimous by method
0e4cc3c [Anthony Corbacho] Add new method to check if user and roles are member of the note (at least owner, reader, writer)
da3415f [Anthony Corbacho] Add new method to help to determinate if user is part of writer and/or owner for the given note
4a43b07 [Anthony Corbacho] Add new method on ZeppelinConfiguration to get is zeppelin is running on anonimous mode or not
commit bbf17da9e5ac272227083fcdafadb13842898cac
Author: hkropp <hk...@hortonworks.com>
Date: 2016-11-03T09:42:04Z
Merge branch 'ZEPPELIN-1611' of github.com:hkropp/incubator-zeppelin into ZEPPELIN-1611
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1589: [Zeppelin-1611] - Support PAM (System User) Authentica...
Posted by bzz <gi...@git.apache.org>.
Github user bzz commented on the issue:
https://github.com/apache/zeppelin/pull/1589
\U0001f44d for docs in same PR.
On CI, I think that's relativly minor and if that's too complicated to configure - we can can opt out for manual test run+instruction in docs.
Double-checking that we have all Licenses for dependencies (and transitive dependencies) logged is important though.
BTW, are you sure that all those commits belong to this branch?
```
@anthonycorbacho [ZEPPELIN-1586] Add security check in NotebookRestApi \u2026 80c5360
@astroshim [ZEPPELIN-1585] Testcase for PySparkInterpreter. \u2026 3f03aa3
@cloverhearts [hotfix] does not showing notebooklist on navbar \u2026 990cc86
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1589: [Zeppelin-1611] - Support PAM (System User) Authentica...
Posted by hkropp <gi...@git.apache.org>.
Github user hkropp commented on the issue:
https://github.com/apache/zeppelin/pull/1589
Provided documentation as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1589: [Zeppelin-1611] - Support PAM (System User) Authentica...
Posted by hkropp <gi...@git.apache.org>.
Github user hkropp commented on the issue:
https://github.com/apache/zeppelin/pull/1589
Cool!
Just to be clear the test will simply be ignored with `assumeTrue` so should be fine without any additional effort. How to use the test itself is documented in the test class with comments.
Concerning the licenses we should be fairly save. libpam4j mainly dependence on JNA and maven. JNA version >4 is available as Apache and actually was already introduced with selenium dependency in this project.
I rebased and pulled in a way that created a little mess here. What I could do is create a new branch with cherry-picked changes and created a new PR based on this, or? Let me know?
I think the Travis build failed because of issue with the other changes, or?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1589: [Zeppelin-1611] - Support PAM (System User) Authentica...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1589
Tested and LGTM.
Merge if there's no further discussion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #1589: [Zeppelin-1611] - Support PAM (System User) Aut...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/1589
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1589: [Zeppelin-1611] - Support PAM (System User) Authentica...
Posted by hkropp <gi...@git.apache.org>.
Github user hkropp commented on the issue:
https://github.com/apache/zeppelin/pull/1589
Thanks for your feedback @bzz
I applied the changes required for 1. and 2.
3. Concerning Travis:
It would need to be a username and password local to the machine the test is being executed, further it might need to be user that can potentially ssh to the machine as the `/etc/pam.d/sshd` is being used. For now I think it would be fine to consider this as a manually test.
4. I'll be happy to provide the documentation. Does this have to be a new PR or can a PR be of multiple types?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1589: [Zeppelin-1611] - Support PAM (System User) Authentica...
Posted by hkropp <gi...@git.apache.org>.
Github user hkropp commented on the issue:
https://github.com/apache/zeppelin/pull/1589
Thank you @bzz and @Leemoonsoo , I rewrote history and added the shiro.ini.template. I think we should be fine here now, or? Please let me know, if you have further remarks.
Just so you are aware, I believe this currently does not support the resolution/listing of users and groups in UI for example for notebook authorization. Usernames and groups will stay empty with this. I created [ZEPPELIN-1631](https://issues.apache.org/jira/browse/ZEPPELIN-1631) as a followup. I already have an idea on how to solve this and might be able to contribute this within next weeks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1589: [Zeppelin-1611] - Support PAM (System User) Authentica...
Posted by bzz <gi...@git.apache.org>.
Github user bzz commented on the issue:
https://github.com/apache/zeppelin/pull/1589
Thank you for contribution!
There are few things need to be taken care here:
1. make sure the code adhere [project styleguide](https://zeppelin.apache.org/contribution/contributions.html#code-convention) - i.e whildcard imports in Java are discouraged, etc
2. make sure that licences for all new runtime dependencies [are compatible](https://www.apache.org/legal/resolved#category-a), and were added added to `./zeppelin-distribution/src/bin_license/LICENSE`
3. AFAIK right now new tests will not be run on TravisC infrastructure as they require some env vars to be set, but no modifications to [.travis.yml](https://github.com/apache/zeppelin/blob/master/.travis.yml). Do you think it's worth adding them to one CI profile?
4. In PR description
> Does this needs documentation? Yes
Do you plan to provide documentation at `./docs` in this PR as well?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---