Skip to content

Conversation

@AhyoungRyu
Copy link
Contributor

@AhyoungRyu AhyoungRyu commented Nov 28, 2016

What is this PR for?

Currently anonymous user can open the notebook permission page and type sth in Owner/ Reader / Writer and then even can save it. However, in fact, it doesn't work actually.

e.g. An anonymous user can type admin / user1 to the note permission setting fields.

It doesn't make sense. At least we should disallow the non-authenticated users(a.k.a anonymous users) by deactivating those permission related features(will handle interpreter owner setting in another PR). So what I did in this PR is

TODO

  • Fix test case

What type of PR is it?

Bug Fix | Improvement

What is the Jira issue?

ZEPPELIN-1718

How should this be tested?

  1. Don't activate shiro authentication
  2. Open any notes
  3. Click lock icon in the top of the note -> warning dialog will be shown \w "Only authenticated user can set the permission." sentence

Screenshots (if appropriate)

  • Doesn't show note permission setting page & show warning dialog if anonymous user tries to click lock icon
    block

Questions:

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

@AhyoungRyu AhyoungRyu closed this Nov 28, 2016
@AhyoungRyu AhyoungRyu reopened this Nov 28, 2016
@prabhjyotsingh
Copy link
Contributor

Haven't tried it yet, but IMO we should have a check at backned for the same (both API and websocket).

@cloverhearts
Copy link
Member

It is a cool feature.
I would like to say one thing.
If you need to prevent configuration changes for anonymous users,
you should not modify them only in the Web UI.

https://github.com/apache/zeppelin/blob/master/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java#L112

https://github.com/apache/zeppelin/blob/master/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java#L134

https://github.com/apache/zeppelin/blob/master/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java # L165

Web ui can be manipulated very easily by the user, so we think that we need to modify the above restful api additionally.

@AhyoungRyu
Copy link
Contributor Author

@prabhjyotsingh @cloverhearts Didn't think about that way. It makes sense. Thanks for your suggestion. Let me update then :)

@astroshim
Copy link
Contributor

astroshim commented Dec 12, 2016

It's working well but I agree with @prabhjyotsingh and @cloverhearts blocking to change the permission on a backend side too.

@AhyoungRyu AhyoungRyu changed the title [ZEPPELIN-1718] Prevent anonymous user to set note permission / interpreter owner [ZEPPELIN-1718] Prevent anonymous user to set note permission Dec 18, 2016
@AhyoungRyu AhyoungRyu force-pushed the prevent-anon-user branch 2 times, most recently from e5d2c47 to 06c6326 Compare December 18, 2016 16:25
@AhyoungRyu AhyoungRyu closed this Dec 19, 2016
@AhyoungRyu AhyoungRyu reopened this Dec 19, 2016
@AhyoungRyu
Copy link
Contributor Author

AhyoungRyu commented Dec 19, 2016

@prabhjyotsingh @cloverhearts @astroshim Sorry for my late response.
I added checking anonymous user using notebook rest api. So if anonymous users try to click lock icon / GET note permission setting info using url (e.g. http://http://localhost:8080/api/notebook/2C5QFHKYM/permissions), then 403 forbidden error will be shown. I updated PR description \w new UI screenshot image accordingly. Please take a look :)

And if you guys don't mind, will handle blocking interpreter owner setting part by anon user in other PR.

@AhyoungRyu
Copy link
Contributor Author

CI is all green now! Ready to review.
Here are some brief summaries what I did.

Since from this PR anonymous(non authenticated) user can't set note permission, related test cases also need to be updated accordingly. Currently we have two test cases which check note permission as anonymous user.

So I rewrote them based on #1567's NotebookSecurityRestApiTest.java to be tested as authenticated users.

@AhyoungRyu
Copy link
Contributor Author

Can someone review this one? 🎅🏼

@prabhjyotsingh
Copy link
Contributor

Tested, works as expectation. LGTM!

@AhyoungRyu
Copy link
Contributor Author

@prabhjyotsingh Thanks! :D

@AhyoungRyu
Copy link
Contributor Author

Will merge this into master if there are no more comments!

@asfgit asfgit closed this in 14a38b7 Dec 28, 2016
@Leemoonsoo
Copy link
Member

After this patch, if i turn off shiro and load any notebook (without click permission button)

browser console print 403 forbidden,
image

And ZeppelinServer prints stack trace

WARN [2017-01-01 12:03:26,784] ({qtp106298691-16} WebApplicationExceptionMapper.java[toResponse]:73) - org.apache.zeppelin.rest.exception.ForbiddenException
	at org.apache.zeppelin.rest.NotebookRestApi.checkIfUserIsAnon(NotebookRestApi.java:133)
	at org.apache.zeppelin.rest.NotebookRestApi.getNotePermissions(NotebookRestApi.java:98)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:606)
	at org.apache.cxf.service.invoker.AbstractInvoker.performInvocation(AbstractInvoker.java:180)
	at org.apache.cxf.service.invoker.AbstractInvoker.invoke(AbstractInvoker.java:96)
	at org.apache.cxf.jaxrs.JAXRSInvoker.invoke(JAXRSInvoker.java:205)
	at org.apache.cxf.jaxrs.JAXRSInvoker.invoke(JAXRSInvoker.java:102)
	at 

Can we try not print error / warning on loading notebook ?

@cloverhearts
Copy link
Member

@Leemoonsoo #1821
I was fixed that issue.

@RoxanaTapia
Copy link

Hi,
I'm experiencing this issue but with shiro activated. I want the "Note Permissions" button to be activated only for authorized users (e.g. admins). Right now any user can access, change and save the configs under "Note Permissions", so that they can edit notebooks.

@AhyoungRyu
Copy link
Contributor Author

@RoxanaTapia Could you file a Jira issue here https://issues.apache.org/jira/projects/ZEPPELIN so that it can be tracked & handled separately ? Once a patch is merged, can't be opened again or tracked anymore.

@RoxanaTapia
Copy link

@AhyoungRyu Thanks for the quick response, here is the JIRA ticket link https://issues.apache.org/jira/browse/ZEPPELIN-3133

@RoxanaTapia
Copy link

RoxanaTapia commented Jan 15, 2018

@AhyoungRyu I'm still experiencing this issue, any idea how can I patch it, using credentials or roles?

@BigDataDataAnalyst
Copy link

BigDataDataAnalyst commented Apr 23, 2018

@AhyoungRyu
I am facing the same issue which was reported earlier by @RoxanaTapia with Zeppelin Notebook Authorisation(with Shiro Enabled).

Here is the JIRA ticket link.
https://issues.apache.org/jira/browse/ZEPPELIN-3133

It would be great if you could throw some insights on options if any available to get-around with this issue

@AhyoungRyu AhyoungRyu deleted the prevent-anon-user branch April 23, 2018 08:44
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.

7 participants