[GitHub] zeppelin issue #1567: [ZEPPELIN-1586] Add security check in NotebookRestApi
Github user anthonycorbacho commented on the issue: https://github.com/apache/zeppelin/pull/1567 Yeaay! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1567: [ZEPPELIN-1586] Add security check in NotebookRestApi
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1567 I guess it's same CI error with #1518. There is a log which is: ```bash gzip: stdin: unexpected end of file tar: Unexpected EOF in archive tar: Unexpected EOF in archive tar: Error is not recoverable: exiting now +echo 'Unable to extract spark-1.3.1-bin-hadoop2.3.tgz' Unable to extract spark-1.3.1-bin-hadoop2.3.tgz ``` This is because of cache failure of Travis CI, and if it happens, it goes forever. Because caching is done only at the first time. Because of this, if you follow the log more, you can see: ```bash SPARK HOME detected null ``` Therefore, Zeppelin cannot find Spark, and it goes to failure. I think it will pass if you run Travis CI on your own repository. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1567: [ZEPPELIN-1586] Add security check in NotebookRestApi
Github user minahlee commented on the issue: https://github.com/apache/zeppelin/pull/1567 Same Spark 1.3.1 test profile failure exists on master which looks like below: ``` Results : Tests in error: InterpreterRestApiTest.init:57->AbstractTestRestApi.startUp:233->AbstractTestRestApi.start:201 û NullPointer CredentialsRestApiTest.init:46->AbstractTestRestApi.startUp:233->AbstractTestRestApi.start:201 û NullPointer ZeppelinRestApiTest.init:59->AbstractTestRestApi.startUp:233->AbstractTestRestApi.start:201 û NullPointer NotebookRestApiTest.init:58->AbstractTestRestApi.startUp:233->AbstractTestRestApi.start:201 û NullPointer SecurityRestApiTest.init:44->AbstractTestRestApi.startUp:233->AbstractTestRestApi.start:201 û NullPointer NotebookRepoRestApiTest.init:52->AbstractTestRestApi.startUp:233->AbstractTestRestApi.start:201 û NullPointer ConfigurationsRestApiTest.init:39->AbstractTestRestApi.startUp:233->AbstractTestRestApi.start:201 û NullPointer ZeppelinSparkClusterTest.init:52->AbstractTestRestApi.startUp:233->AbstractTestRestApi.start:201 û NullPointer NotebookSecurityRestApiTest.init:49->AbstractTestRestApi.startUpWithAuthenticationEnable:229->AbstractTestRestApi.start:201 û NullPointer Tests run: 9, Failures: 0, Errors: 9, Skipped: 0 ``` So I am going to merge this if there is no more 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1567: [ZEPPELIN-1586] Add security check in NotebookRestApi
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1567 @anthonycorbacho Thanks :-) Don't worry! I do this because I want to do ð --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1567: [ZEPPELIN-1586] Add security check in NotebookRestApi
Github user anthonycorbacho commented on the issue: https://github.com/apache/zeppelin/pull/1567 @tae-jun creating an issue doenst mean that you have to handle it, of course if you want you are welcome to do so, but remember we are a community so that mean we are here to help each others so if you create the PR i can spend some time and work with you on it :) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1567: [ZEPPELIN-1586] Add security check in NotebookRestApi
Github user anthonycorbacho commented on the issue: https://github.com/apache/zeppelin/pull/1567 @zjffdu Hey dudy I keep failing at Livy interpreter, i dont know why but i keep getting timeout somehow, can you take a look t it please? It also look like other pr are failing at the same stage :( --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1567: [ZEPPELIN-1586] Add security check in NotebookRestApi
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1567 @anthonycorbacho Nice! I agree with you :) I will open the issue on JIRA. But since I don't know much about the code structure, I may need some help ð --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1567: [ZEPPELIN-1586] Add security check in NotebookRestApi
Github user anthonycorbacho commented on the issue: https://github.com/apache/zeppelin/pull/1567 @tae-jun After looking at the code base, I think this case should be handle in another PR, this is kinda out of the scope of this PR and its already becoming super big. But your made a very valid point here and I guess it deserve a Jira ticket. i will try to find some time to handle this special case. What do you think? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1567: [ZEPPELIN-1586] Add security check in NotebookRestApi
Github user anthonycorbacho commented on the issue: https://github.com/apache/zeppelin/pull/1567 @tae-jun thanks for the feedback, let me take a look tomorrow --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1567: [ZEPPELIN-1586] Add security check in NotebookRestApi
Github user tae-jun commented on the issue:
https://github.com/apache/zeppelin/pull/1567
I also tested and was able to reproduce screenshot above.
When I was another user, it returned `403` status code with message:
```json
{"status":"FORBIDDEN","message":"Insufficient privileges you cannot get
this note"}
```
However, when I didn't log in (i.e. anonymous), the browser(Chrome)
redirected me to `http://localhost:8080/api/login` and returned `405` status
code without any message. Users can be confused when there is no error message.
And I think `403` status code is more proper since it's `forbidden`, not
`method not allowed`.
So in my opinion, it would be better:
- Send `403` status code with some messages when a user is not logged in.
Maybe something like:
```json
{"status":"FORBIDDEN","message":"Please log in"}
```
This is a miracle feature, by the way ð
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1567: [ZEPPELIN-1586] Add security check in NotebookRestApi
Github user anthonycorbacho commented on the issue: https://github.com/apache/zeppelin/pull/1567 @minahlee you are right, I guess the next step will be to abstract this logic from rest api and apply to both rest and websocket. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1567: [ZEPPELIN-1586] Add security check in NotebookRestApi
Github user minahlee commented on the issue: https://github.com/apache/zeppelin/pull/1567 I tested some of rest apis and it works well. Next step would be applying same policies to websocket. For example `reader` cannot change bound interpreter to note via rest api after this PR, but it is possible to do it via websocket(or GUI). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1567: [ZEPPELIN-1586] Add security check in NotebookRestApi
Github user anthonycorbacho commented on the issue: https://github.com/apache/zeppelin/pull/1567 @minahlee yeah, actually i am doing this right now, I also updated to todo tasks thanks for your review! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1567: [ZEPPELIN-1586] Add security check in NotebookRestApi
Github user minahlee commented on the issue: https://github.com/apache/zeppelin/pull/1567 Thank you for quick response, I only went through the code and it looks good to me. It would be nice if you can add some tests. Meanwhile let me build this branch and test it out. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
