Skip to content

[ZEPPELIN-1237] Auto-suggestion of notebook permissions should list roles as well#1236

Closed
prabhjyotsingh wants to merge 3 commits intoapache:masterfrom
prabhjyotsingh:ZEPPELIN-1237
Closed

[ZEPPELIN-1237] Auto-suggestion of notebook permissions should list roles as well#1236
prabhjyotsingh wants to merge 3 commits intoapache:masterfrom
prabhjyotsingh:ZEPPELIN-1237

Conversation

@prabhjyotsingh
Copy link
Contributor

What is this PR for?

Auto-suggestion of notebook permissions should list roles as well

What type of PR is it?

[Improvement]

Todos

  • - Fix test case (selenium)
  • - select2 in bower.json

What is the Jira issue?

How should this be tested?

Search for group/roles in notebook permission, it should get listed

Screenshots (if appropriate)

screen shot 2016-07-27 at 7 13 44 pm

Questions:

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

import org.apache.zeppelin.server.JsonResponse;
import org.apache.zeppelin.ticket.TicketContainer;
import org.apache.zeppelin.utils.SecurityUtils;
import org.eclipse.jetty.util.StringUtil;
Copy link
Member

Choose a reason for hiding this comment

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

unused

@r-kamath
Copy link
Member

@prabhjyotsingh 404 for /userlist api call when clicked on text fields.
user: anonymous
screen shot 2016-07-27 at 10 46 03 pm

@prabhjyotsingh
Copy link
Contributor Author

@r-kamath have implement your feedback.

@felixcheung
Copy link
Member

is it a bit of an information disclosure security issue though, that we should all user name when someone type the first character?

@prabhjyotsingh
Copy link
Contributor Author

I've looked up few products of which the popular ones Outlook (app) limit to 8 results, Outlook (web i.e. owa) limit to 5 results and JIRA does not limits.

I don't have a strong opinion on this, can change according to recommendation.

@rconline
Copy link
Contributor

rconline commented Aug 1, 2016

@felixcheung i think this will continue to remain a problem, till the time we allow all user roles to setup/modify these permissions. Unless we can recognize and separate this as an admin (superuser) action, this seems okay to me. There will be cases when the LDAP user id's will end up in zeppelin/POSIX systems as 'a1890xx', based upon conversion rules, when users who setup these permissions will find it very hard to use. Hence for now, showing all users is a reasonable approach.

@felixcheung
Copy link
Member

I'm ok with it - maybe open a JIRA to track role-based access control on user names?

@prabhjyotsingh
Copy link
Contributor Author

I think a part of this problem is solved by #993 (ZEPPELIN-987). But, yes I agree this has scope for improvising.

Will merge this if no more discussion.

@asfgit asfgit closed this in 1d0028b Aug 4, 2016
PhilippGrulich pushed a commit to SWC-SENSE/zeppelin that referenced this pull request Aug 8, 2016
…oles as well

### What is this PR for?
Auto-suggestion of notebook permissions should list roles as well

### What type of PR is it?
[Improvement]

### Todos
* [x] - Fix test case (selenium)
* [x] - select2 in bower.json

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

### How should this be tested?
Search for group/roles in notebook permission, it should get listed

### Screenshots (if appropriate)
![screen shot 2016-07-27 at 7 13 44 pm](https://cloud.githubusercontent.com/assets/674497/17177288/4f5ac594-542e-11e6-8543-d62c238e8105.png)

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

Author: Prabhjyot Singh <prabhjyotsingh@gmail.com>

Closes apache#1236 from prabhjyotsingh/ZEPPELIN-1237 and squashes the following commits:

b944dc2 [Prabhjyot Singh] Merge remote-tracking branch 'origin/master' into ZEPPELIN-1237
17e17a9 [Prabhjyot Singh] implement @r-kamath feedback
0793c10 [Prabhjyot Singh] Auto-suggestion of notebook permissions should list group as well
@prabhjyotsingh prabhjyotsingh deleted the ZEPPELIN-1237 branch February 25, 2018 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants