Skip to content

[ZEPPELIN-987] Enable user to secure interpreter setting, credentials and configurations info#993

Closed
AhyoungRyu wants to merge 7 commits intoapache:masterfrom
AhyoungRyu:ZEPPELIN-987
Closed

[ZEPPELIN-987] Enable user to secure interpreter setting, credentials and configurations info#993
AhyoungRyu wants to merge 7 commits intoapache:masterfrom
AhyoungRyu:ZEPPELIN-987

Conversation

@AhyoungRyu
Copy link
Copy Markdown
Contributor

@AhyoungRyu AhyoungRyu commented Jun 10, 2016 •
edited
Loading

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
Copy Markdown
Contributor

prabhjyotsingh commented Jun 15, 2016

@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
Copy Markdown
Contributor Author

AhyoungRyu commented Jun 15, 2016

@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
Copy Markdown
Contributor

prabhjyotsingh commented Jun 15, 2016

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
Copy Markdown
Contributor Author

AhyoungRyu commented Jun 15, 2016

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

@echarles
Copy link
Copy Markdown
Member

echarles commented Jun 15, 2016

@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
Copy Markdown
Contributor Author

AhyoungRyu commented Jun 15, 2016

@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
Copy Markdown
Contributor

corneadoug commented Jul 4, 2016

@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
Copy Markdown
Contributor Author

AhyoungRyu commented Jul 9, 2016 •
edited
Loading

@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
Copy Markdown
Contributor

prabhjyotsingh commented Jul 10, 2016

Sure this will work. Thanks for the fix.

prabhjyotsingh
prabhjyotsingh reviewed Jul 10, 2016
View reviewed changes
conf/shiro.ini
@@ -60,10 +60,14 @@ role2 = *
role3 = *

Copy link
Copy Markdown
Contributor

@prabhjyotsingh prabhjyotsingh Jul 10, 2016

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
Copy Markdown
Contributor Author

AhyoungRyu commented Jul 10, 2016 •
edited
Loading

@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
Copy Markdown
Contributor

prabhjyotsingh commented Jul 10, 2016

LGTM!

@AhyoungRyu
Copy link
Copy Markdown
Contributor Author

AhyoungRyu commented Jul 10, 2016

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

@AhyoungRyu
Copy link
Copy Markdown
Contributor Author

AhyoungRyu commented Jul 11, 2016

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

@prabhjyotsingh
Copy link
Copy Markdown
Contributor

prabhjyotsingh commented Jul 11, 2016

Tested on both firefox and chrome. Works well. 👍

@prabhjyotsingh
Copy link
Copy Markdown
Contributor

prabhjyotsingh commented Jul 12, 2016

Merging this if no more discussion.

@asfgit asfgit closed this in 1a2cced Jul 14, 2016
@prabhjyotsingh prabhjyotsingh mentioned this pull request Aug 2, 2016
Closed
2 tasks
PhilippGrulich pushed a commit to SWC-SENSE/zeppelin that referenced this pull request Aug 8, 2016
@AhyoungRyu @PhilippGrulich
[ZEPPELIN-987] Enable user to secure interpreter setting, credentials…
c2db821 
… 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 <[email protected]>

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@AhyoungRyu @prabhjyotsingh @echarles @corneadoug