Uploaded image for project: 'Zeppelin'
  1. Zeppelin
  2. ZEPPELIN-1144

Zeppelin home page should only list notebooks with read or write permission

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 0.6.0
    • Fix Version/s: 0.7.0
    • Component/s: None
    • Labels:
      None

      Description

      Scenario:

      • enable zeppelin authentication
      • Log in as user : admin
      • Create notebook ( admin_user1)
      • set reader =admin and writer = admin
      • In seperate window, log in as user : hrt_1
      • Check the zeppelin welcome page.

      Actual behavior: admin_user1 notebook is listed in Zeppelin home page

      Expected : If logged in user does not have Read and Write permission for a notebook, user should not see the notebook in the zeppelin home page.

      If we do not fix this issue, each user will be able to see all other user's notebook list. That is not correct behavior.

        Issue Links

          Activity

          Hide
          minwoo.kang Minwoo Kang added a comment -

          I can handle this issue.

          Show
          minwoo.kang Minwoo Kang added a comment - I can handle this issue.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user mwkang opened a pull request:

          https://github.com/apache/zeppelin/pull/1330

          ZEPPELIN-1144Zeppelin home page should only list notebooks with read or write permission

              1. What is this PR for?
                If logged in user does not have Read and Write permission for a notebook, user should not see the notebook in the zeppelin home page.
              1. What type of PR is it?
                Bug Fix
              1. Todos
              1. What is the Jira issue?
                https://issues.apache.org/jira/browse/ZEPPELIN-1144
              1. How should this be tested?
          • unit test
          • online test
              1. Screenshots (if appropriate)
              1. Questions:
          • Does the licenses files need update? no
          • Is there breaking changes for older versions? no
          • Does this needs documentation? no

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/mwkang/zeppelin ZEPPELIN-1144

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/zeppelin/pull/1330.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #1330


          commit 0a011db3ac5314e62250058459460d44c68551e2
          Author: Minwoo Kang <minwoo.kang@outlook.com>
          Date: 2016-08-13T07:49:32Z

          User see read, write, owner permission notebook.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user mwkang opened a pull request: https://github.com/apache/zeppelin/pull/1330 ZEPPELIN-1144 Zeppelin home page should only list notebooks with read or write permission What is this PR for? If logged in user does not have Read and Write permission for a notebook, user should not see the notebook in the zeppelin home page. What type of PR is it? Bug Fix Todos What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-1144 How should this be tested? unit test online test Screenshots (if appropriate) Questions: Does the licenses files need update? no Is there breaking changes for older versions? no Does this needs documentation? no You can merge this pull request into a Git repository by running: $ git pull https://github.com/mwkang/zeppelin ZEPPELIN-1144 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zeppelin/pull/1330.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1330 commit 0a011db3ac5314e62250058459460d44c68551e2 Author: Minwoo Kang <minwoo.kang@outlook.com> Date: 2016-08-13T07:49:32Z User see read, write, owner permission notebook.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mwkang commented on the issue:

          https://github.com/apache/zeppelin/pull/1330

          CI error
          ```
          Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 76.102 sec - in org.apache.zeppelin.integration.SparkParagraphIT

          Results :

          Tests in error:
          ZeppelinIT.testAngularDisplay:118->AbstractZeppelinIT.waitForParagraph:70->AbstractZeppelinIT.pollingWait:96 쨩 Timeout
          AuthenticationIT.testGroupPermission:179->AbstractZeppelinIT.pollingWait:96 쨩 Timeout

          Tests run: 15, Failures: 0, Errors: 2, Skipped: 0
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user mwkang commented on the issue: https://github.com/apache/zeppelin/pull/1330 CI error ``` Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 76.102 sec - in org.apache.zeppelin.integration.SparkParagraphIT Results : Tests in error: ZeppelinIT.testAngularDisplay:118->AbstractZeppelinIT.waitForParagraph:70->AbstractZeppelinIT.pollingWait:96 쨩 Timeout AuthenticationIT.testGroupPermission:179->AbstractZeppelinIT.pollingWait:96 쨩 Timeout Tests run: 15, Failures: 0, Errors: 2, Skipped: 0 ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user bzz commented on the issue:

          https://github.com/apache/zeppelin/pull/1330

          Thank you for fixing it @mwkang

          CI failure looks somehow relevant, you can not reproduce it locally?

          Show
          githubbot ASF GitHub Bot added a comment - Github user bzz commented on the issue: https://github.com/apache/zeppelin/pull/1330 Thank you for fixing it @mwkang CI failure looks somehow relevant, you can not reproduce it locally?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mwkang commented on the issue:

          https://github.com/apache/zeppelin/pull/1330

          @bzz I will check it. Thanks for review.
          When I fix CI, I'll mention you.

          Show
          githubbot ASF GitHub Bot added a comment - Github user mwkang commented on the issue: https://github.com/apache/zeppelin/pull/1330 @bzz I will check it. Thanks for review. When I fix CI, I'll mention you.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mwkang commented on the issue:

          https://github.com/apache/zeppelin/pull/1330

          @bzz https://github.com/apache/zeppelin/pull/1330#issuecomment-239997747 It did not reproduce in local. Can you check this?

          Show
          githubbot ASF GitHub Bot added a comment - Github user mwkang commented on the issue: https://github.com/apache/zeppelin/pull/1330 @bzz https://github.com/apache/zeppelin/pull/1330#issuecomment-239997747 It did not reproduce in local. Can you check this?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user bzz commented on the issue:

          https://github.com/apache/zeppelin/pull/1330

          @mwkang could you please try close and then re-open this PR to trigger the CI and see if the same issue persists? Thanks!

          \cc @khalidhuseynov for review.

          Show
          githubbot ASF GitHub Bot added a comment - Github user bzz commented on the issue: https://github.com/apache/zeppelin/pull/1330 @mwkang could you please try close and then re-open this PR to trigger the CI and see if the same issue persists? Thanks! \cc @khalidhuseynov for review.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user khalidhuseynov commented on the issue:

          https://github.com/apache/zeppelin/pull/1330

          @mwkang thanks for the contribution! actually this is related to multi-user note management and there's some work going on under #1392 or #1390 .

          In your case, i tested it, and there's a bit tricky moment with notes reloading and broadcasting. For example if you login as user1 with noteA and user2 with noteB and initially everything looks as expected, and then you reload your notes as shown below with refresh icon:
          <img width="305" alt="screen shot 2016-09-02 at 5 52 10 pm" src="https://cloud.githubusercontent.com/assets/1642088/18198247/737174ee-7136-11e6-9d94-ba75f6882981.png">
          then if you refresh for user1 then noteA will appear in the list of all users, including user2 in our case. Same would happen if you reload for user2, meaning noteB will appear in the list of user1.

          Show
          githubbot ASF GitHub Bot added a comment - Github user khalidhuseynov commented on the issue: https://github.com/apache/zeppelin/pull/1330 @mwkang thanks for the contribution! actually this is related to multi-user note management and there's some work going on under #1392 or #1390 . In your case, i tested it, and there's a bit tricky moment with notes reloading and broadcasting. For example if you login as user1 with noteA and user2 with noteB and initially everything looks as expected, and then you reload your notes as shown below with refresh icon: <img width="305" alt="screen shot 2016-09-02 at 5 52 10 pm" src="https://cloud.githubusercontent.com/assets/1642088/18198247/737174ee-7136-11e6-9d94-ba75f6882981.png"> then if you refresh for user1 then noteA will appear in the list of all users, including user2 in our case. Same would happen if you reload for user2, meaning noteB will appear in the list of user1.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user echarles commented on the issue:

          https://github.com/apache/zeppelin/pull/1330

          If the current single `notebook-authorization.json` is managed by user (with multiple files, and no more with a single one), how would you handle it?

          Show
          githubbot ASF GitHub Bot added a comment - Github user echarles commented on the issue: https://github.com/apache/zeppelin/pull/1330 If the current single `notebook-authorization.json` is managed by user (with multiple files, and no more with a single one), how would you handle it?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user khalidhuseynov commented on the issue:

          https://github.com/apache/zeppelin/pull/1330

          @mwkang could you re-trigger CI?

          Show
          githubbot ASF GitHub Bot added a comment - Github user khalidhuseynov commented on the issue: https://github.com/apache/zeppelin/pull/1330 @mwkang could you re-trigger CI?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user khalidhuseynov commented on the issue:

          https://github.com/apache/zeppelin/pull/1330

          I think it doesn't solve all problems, but definitely in the roadmap. LGTM

          Show
          githubbot ASF GitHub Bot added a comment - Github user khalidhuseynov commented on the issue: https://github.com/apache/zeppelin/pull/1330 I think it doesn't solve all problems, but definitely in the roadmap. LGTM
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user bzz commented on the issue:

          https://github.com/apache/zeppelin/pull/1330

          Thank you @mwkang @khalidhuseynov !

          Will merge to master, if there is no further discussion.

          Show
          githubbot ASF GitHub Bot added a comment - Github user bzz commented on the issue: https://github.com/apache/zeppelin/pull/1330 Thank you @mwkang @khalidhuseynov ! Will merge to master, if there is no further discussion.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Leemoonsoo commented on the issue:

          https://github.com/apache/zeppelin/pull/1330

          @mwkang @khalidhuseynov Could you handle or at least create issues for expected problems after merge this PR? I can at least think following two issues,

          • Note list broadcasting message is not aware user session, so notebook list can display notes that supposed to not see when other user refresh the notebook list.
          • When a user changes the permission of a note, note list need to be broadcasted immediately so all connected users can have updated list of notebook. Otherwise, there's still some chances that a user able to see some note on notebook list that they can not access.
          Show
          githubbot ASF GitHub Bot added a comment - Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/1330 @mwkang @khalidhuseynov Could you handle or at least create issues for expected problems after merge this PR? I can at least think following two issues, Note list broadcasting message is not aware user session, so notebook list can display notes that supposed to not see when other user refresh the notebook list. When a user changes the permission of a note, note list need to be broadcasted immediately so all connected users can have updated list of notebook. Otherwise, there's still some chances that a user able to see some note on notebook list that they can not access.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user khalidhuseynov commented on the issue:

          https://github.com/apache/zeppelin/pull/1330

          @Leemoonsoo issues are created under ZEPPELIN-1437(https://issues.apache.org/jira/browse/ZEPPELIN-1437) and ZEPPELIN-1438(https://issues.apache.org/jira/browse/ZEPPELIN-1438) correspondingly

          Show
          githubbot ASF GitHub Bot added a comment - Github user khalidhuseynov commented on the issue: https://github.com/apache/zeppelin/pull/1330 @Leemoonsoo issues are created under ZEPPELIN-1437 ( https://issues.apache.org/jira/browse/ZEPPELIN-1437 ) and ZEPPELIN-1438 ( https://issues.apache.org/jira/browse/ZEPPELIN-1438 ) correspondingly
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Leemoonsoo commented on the issue:

          https://github.com/apache/zeppelin/pull/1330

          Thanks, Merge to master if there're no further discussions.

          Show
          githubbot ASF GitHub Bot added a comment - Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/1330 Thanks, Merge to master if there're no further discussions.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/zeppelin/pull/1330

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/zeppelin/pull/1330
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user AhyoungRyu opened a pull request:

          https://github.com/apache/zeppelin/pull/1432

          [HOT FIX]ZEPPELIN-1144 Fix compilation errors in Notebook.java

              1. What is this PR for?
                After #1330 merged, the latest master build failed with below compilation errors.

          ```
          [ERROR] COMPILATION ERROR :
          [INFO] -------------------------------------------------------------
          [ERROR] /Users/ahyoungryu/Dev/zeppelin/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java:[553,31] cannot find symbol
          symbol: method id()
          location: variable note1 of type org.apache.zeppelin.notebook.Note
          [ERROR] /Users/ahyoungryu/Dev/zeppelin/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java:[557,31] cannot find symbol
          symbol: method id()
          location: variable note2 of type org.apache.zeppelin.notebook.Note
          ```

              1. What type of PR is it?
                Hot Fix
              1. What is the Jira issue?
              1. How should this be tested?
          • Build the latest master branch with `mvn clean package -DskipTests` -> compilation error in `zeppelin-zengine`
          • Apply this patch and build with `mvn clean package -DskipTests` -> build success
            You can also check #1330 works properly.
              1. Questions:
          • Does the licenses files need update? no
          • Is there breaking changes for older versions? no
          • Does this needs documentation? no

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/AhyoungRyu/zeppelin hotfix/ZEPPELIN-1144

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/zeppelin/pull/1432.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #1432


          commit 6a3dbd3a33a4a0ffe2614946b243dad63c78e241
          Author: AhyoungRyu <fbdkdud93@hanmail.net>
          Date: 2016-09-17T02:42:36Z

          Fix build error in Notebook.java


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user AhyoungRyu opened a pull request: https://github.com/apache/zeppelin/pull/1432 [HOT FIX] ZEPPELIN-1144 Fix compilation errors in Notebook.java What is this PR for? After #1330 merged, the latest master build failed with below compilation errors. ``` [ERROR] COMPILATION ERROR : [INFO] ------------------------------------------------------------- [ERROR] /Users/ahyoungryu/Dev/zeppelin/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java: [553,31] cannot find symbol symbol: method id() location: variable note1 of type org.apache.zeppelin.notebook.Note [ERROR] /Users/ahyoungryu/Dev/zeppelin/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java: [557,31] cannot find symbol symbol: method id() location: variable note2 of type org.apache.zeppelin.notebook.Note ``` What type of PR is it? Hot Fix What is the Jira issue? How should this be tested? Build the latest master branch with `mvn clean package -DskipTests` -> compilation error in `zeppelin-zengine` Apply this patch and build with `mvn clean package -DskipTests` -> build success You can also check #1330 works properly. Questions: Does the licenses files need update? no Is there breaking changes for older versions? no Does this needs documentation? no You can merge this pull request into a Git repository by running: $ git pull https://github.com/AhyoungRyu/zeppelin hotfix/ ZEPPELIN-1144 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zeppelin/pull/1432.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1432 commit 6a3dbd3a33a4a0ffe2614946b243dad63c78e241 Author: AhyoungRyu <fbdkdud93@hanmail.net> Date: 2016-09-17T02:42:36Z Fix build error in Notebook.java
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user khalidhuseynov commented on the issue:

          https://github.com/apache/zeppelin/pull/1432

          looks good, thanks for quick fix!

          Show
          githubbot ASF GitHub Bot added a comment - Github user khalidhuseynov commented on the issue: https://github.com/apache/zeppelin/pull/1432 looks good, thanks for quick fix!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Leemoonsoo commented on the issue:

          https://github.com/apache/zeppelin/pull/1432

          LGTM

          Show
          githubbot ASF GitHub Bot added a comment - Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/1432 LGTM
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user AhyoungRyu commented on the issue:

          https://github.com/apache/zeppelin/pull/1432

          @khalidhuseynov @Leemoonsoo Thanks for the quick verification. Then will merge this one as a hotfix after CI checking!

          Show
          githubbot ASF GitHub Bot added a comment - Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1432 @khalidhuseynov @Leemoonsoo Thanks for the quick verification. Then will merge this one as a hotfix after CI checking!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user AhyoungRyu commented on the issue:

          https://github.com/apache/zeppelin/pull/1432

          Only Selenium test failed now. I'm merging this as a hotfix.

          Show
          githubbot ASF GitHub Bot added a comment - Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1432 Only Selenium test failed now. I'm merging this as a hotfix.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user prabhjyotsingh commented on the issue:

          https://github.com/apache/zeppelin/pull/1432

          Tested on local, LGTM.

          Show
          githubbot ASF GitHub Bot added a comment - Github user prabhjyotsingh commented on the issue: https://github.com/apache/zeppelin/pull/1432 Tested on local, LGTM.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zjffdu commented on the issue:

          https://github.com/apache/zeppelin/pull/1432

          @prabhjyotsingh @AhyoungRyu This make the compilation fails, which affect other PRs. Could you help do a quick fix on it ?
          https://travis-ci.org/apache/zeppelin/jobs/160614161

          Show
          githubbot ASF GitHub Bot added a comment - Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/1432 @prabhjyotsingh @AhyoungRyu This make the compilation fails, which affect other PRs. Could you help do a quick fix on it ? https://travis-ci.org/apache/zeppelin/jobs/160614161
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zjffdu commented on the issue:

          https://github.com/apache/zeppelin/pull/1432

          Please ignore my previous comments, just realize this is for fix the compilation issue.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/1432 Please ignore my previous comments, just realize this is for fix the compilation issue.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/zeppelin/pull/1432

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/zeppelin/pull/1432
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mwkang commented on the issue:

          https://github.com/apache/zeppelin/pull/1330

          I'm sorry for no-reply long time. my plate was full.
          I will finish other 2 PRs and then If ZEPPELIN-1437 and ZEPPELIN-1438 are status is unresolved, I will
          try to resolve.
          If is it okay..

          Show
          githubbot ASF GitHub Bot added a comment - Github user mwkang commented on the issue: https://github.com/apache/zeppelin/pull/1330 I'm sorry for no-reply long time. my plate was full. I will finish other 2 PRs and then If ZEPPELIN-1437 and ZEPPELIN-1438 are status is unresolved, I will try to resolve. If is it okay..

            People

            • Assignee:
              minwoo.kang Minwoo Kang
              Reporter:
              yeshavora Yesha Vora
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development