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-987] Enable user to secure interpreter setting, credentials and configurations info #993

Closed
wants to merge 7 commits into from

Conversation

AhyoungRyu
Copy link
Contributor

@AhyoungRyu AhyoungRyu commented Jun 10, 2016

What is this PR for?

For some user cases, people might want to hide Interpreter Setting, Credentials and Configurations information to other users (who are defined in conf/shiro.ini). So I added

#/api/interpreter/** = authc, roles[admin]
#/api/configurations/** = authc, roles[admin]
#/api/credential/** = authc, roles[admin]

below the [urls] section.

This issue was originally suggested at Zeppelin user mailing list by @TomNorden

What type of PR is it?

Improvement | Documentation

Todos

  • - Add interpreter, credential and configuration url to conf/shiro.ini
  • - Update shiroauthentication.md for this change
  • - Redirect to home with ngToast error message when status is 401
  • - Rebase after [ZEPPELIN-1054] Improve "Credentials" UI  #1100 merged and add error message to Credential menu as well

What is the Jira issue?

ZEPPELIN-987

How should this be tested?

  1. Apply this patch and restart Zeppelin
  2. Login with admin and password1
  3. Go to interpreter, credential and configuration tab -> You can see all of the information in each tabs
  4. Logout -> Login again with user1 and password2
  5. Go to interpreter, credential and configuration tab -> In this time, you can't see all of the information in each tabs

Screenshots (if appropriate)

  • When you login with user1 (doesn't have permission to see the interpreter, credential and cofiguration info)
    • interpreter menu
      interpreters
    • configuration menu
      configurations
    • credential menu
      credential-after
  • shiroauthentication.md
    screen shot 2016-06-10 at 12 25 02 pm

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? Yes, so I updated.

@AhyoungRyu AhyoungRyu force-pushed the ZEPPELIN-987 branch 2 times, most recently from a4d9dbf to 707527a Compare June 12, 2016 21:01
@prabhjyotsingh
Copy link
Contributor

@AhyoungRyu is this still a work in progress ?
Is there a way in which we can go away with seeing all of these three munu i.e. Interpreter setting, Credentials and Configurations if user is not authorize to ?

Otherwise LGTM.

@AhyoungRyu
Copy link
Contributor Author

@prabhjyotsingh No, I'm just waiting some reviews. I'm not sure that i understood your comment correctly,

Is there a way in which we can go away with seeing all of these three menu i.e. Interpreter setting, Credentials and Configurations if user is not authorize to ?

I just wanted to make even if some users are already defined in [users] section, the other user ( maybe admin) can hide the three information. This is the scope of this PR :)

@prabhjyotsingh
Copy link
Contributor

I was thinking if all of these 3 Interpreter, Credentials, and Configuration menu can be hidden, like the way they are hidden in case of user not loggedin.

@AhyoungRyu
Copy link
Contributor Author

@prabhjyotsingh Yeah it makes sense. It would be better. Let me figure out it then :)

@echarles
Copy link
Member

@AhyoungRyu If you can come to an implementation that fits @prabhjyotsingh good idea, it would be great if you could make it generic, with a configurable list of functions that reflect in to the menu.

@AhyoungRyu
Copy link
Contributor Author

@echarles Yes. Thanks for your suggestion ! :)

@AhyoungRyu AhyoungRyu changed the title [ZEPPELIN-987] Enable user to secure interpreter setting, credentials and configurations info [ZEPPELIN-987][WIP] Enable user to secure interpreter setting, credentials and configurations info Jun 16, 2016
@corneadoug
Copy link
Contributor

@AhyoungRyu Still working on this?
Could you rebase the branch with master?

@AhyoungRyu AhyoungRyu force-pushed the ZEPPELIN-987 branch 3 times, most recently from f2c6111 to b215364 Compare July 9, 2016 14:57
@AhyoungRyu
Copy link
Contributor Author

AhyoungRyu commented Jul 9, 2016

@prabhjyotsingh Sorry for my late response. Even if they don't have permission to those menus(Interpretes, Configurations and Credentials), i think they need to know the existence of the menus. So I added ngToast error message instead of totally hiding the menus. See the updated gif images in the PR description. What do you think?

@prabhjyotsingh
Copy link
Contributor

Sure this will work. Thanks for the fix.

@@ -60,10 +60,14 @@ role2 = *
role3 = *

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add one more role admin i.e. admin = * here, so that other user can also use that role.

@AhyoungRyu
Copy link
Contributor Author

AhyoungRyu commented Jul 10, 2016

@prabhjyotsingh Thanks for your feedback. I addressed it :)

@AhyoungRyu AhyoungRyu changed the title [ZEPPELIN-987][WIP] Enable user to secure interpreter setting, credentials and configurations info [ZEPPELIN-987] Enable user to secure interpreter setting, credentials and configurations info Jul 10, 2016
@prabhjyotsingh
Copy link
Contributor

LGTM!

@AhyoungRyu
Copy link
Contributor Author

@prabhjyotsingh i'll rebase after #1100 merged and add the alert message to credential page as well :)

@AhyoungRyu
Copy link
Contributor Author

@prabhjyotsingh Since #1100 was merged into master, I updated credential.controler.js.

@prabhjyotsingh
Copy link
Contributor

Tested on both firefox and chrome. Works well. 👍

@prabhjyotsingh
Copy link
Contributor

Merging this if no more discussion.

@asfgit asfgit closed this in 1a2cced Jul 14, 2016
PhilippGrulich pushed a commit to SWC-SENSE/zeppelin that referenced this pull request Aug 8, 2016
… and configurations info

### What is this PR for?
For some user cases, people might want to hide **Interpreter Setting**, **Credentials** and **Configurations** information to other users (who are defined in `conf/shiro.ini`). So I added

```
#/api/interpreter/** = authc, roles[admin]
#/api/configurations/** = authc, roles[admin]
#/api/credential/** = authc, roles[admin]
```
below the [ [urls] ](https://github.com/apache/zeppelin/blob/master/conf/shiro.ini#L38) section.

This issue was originally suggested at [Zeppelin user mailing list](https://mail-archives.apache.org/mod_mbox/zeppelin-users/201606.mbox/%3CCAPgU7Y%3DBJrXQ_P0ond4PTukoya0FEjwoPuUb31iN3qwo8iyM1Q%40mail.gmail.com%3E) by TomNorden

### What type of PR is it?
Improvement | Documentation

### Todos
* [x] - Add `interpreter`, `credential` and `configuration` url to `conf/shiro.ini`
* [x] - Update `shiroauthentication.md` for this change
* [x] - Redirect to home with ngToast error message when status is `401`
* [x] - Rebase after apache#1100 merged and add error message to `Credential` menu as well

### What is the Jira issue?
[ZEPPELIN-987](https://issues.apache.org/jira/browse/ZEPPELIN-987)

### How should this be tested?
1. Apply this patch and restart Zeppelin
2. Login with `admin` and `password1`
3. Go to interpreter, credential and configuration tab -> You can see all of the information in each tabs
4. Logout -> Login again with `user1` and `password2`
5. Go to interpreter, credential and configuration tab -> In this time, you can't see all of the information in each tabs

### Screenshots (if appropriate)
- When you login with `user1` (doesn't have permission to see the interpreter, credential and cofiguration info)
  - interpreter menu
![interpreters](https://cloud.githubusercontent.com/assets/10060731/16708520/bedc8732-4631-11e6-938c-ff41d1fbab93.gif)
  - configuration menu
![configurations](https://cloud.githubusercontent.com/assets/10060731/16708525/ce5eb7c0-4631-11e6-9f36-8b97e2b7914a.gif)
  - credential menu
![credential-after](https://cloud.githubusercontent.com/assets/10060731/16726180/e56cfa52-4795-11e6-9a5d-740681092e96.gif)

- `shiroauthentication.md`
<img width="807" alt="screen shot 2016-06-10 at 12 25 02 pm" src="https://cloud.githubusercontent.com/assets/10060731/15976949/a49bc542-2f0a-11e6-8869-8575ba8f1875.png">

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? Yes, so I updated.

Author: AhyoungRyu <fbdkdud93@hanmail.net>

Closes apache#993 from AhyoungRyu/ZEPPELIN-987 and squashes the following commits:

1d291ac [AhyoungRyu] Redirect to home when unauthorized user click 'credentials'
5896c12 [AhyoungRyu] Revert shiro setting
4411188 [AhyoungRyu] Address @prabhjyotsingh feedback
5c9242c [AhyoungRyu] Redirect to home with error message when status is 401
2a054d4 [AhyoungRyu] Add interpreter, credential and configuration urls to shiro.ini
d3a81d5 [AhyoungRyu] Update shiro authentication docs
8be7970 [AhyoungRyu] Change authcBasic -> authc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants