[ZEPPELIN-6556] Personalized mode leaks a non-owner's paragraph edits into the shared master paragraph

zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java

@@ -33,6 +33,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -452,14 +453,19 @@ public boolean runParagraph(Note note,
       callback.onFailure(new IOException("paragraph is disabled."), context);
       return false;
     }
-    p.setText(text);
-    p.setTitle(title);
-    p.setAuthenticationInfo(context.getAutheInfo());
-    if (params != null && !params.isEmpty()) {
-      p.settings.setParams(params);
-    }
-    if (config != null && !config.isEmpty()) {
-      p.mergeConfig(config);
+    // In personalized mode only the note owner may update the master paragraph, so that
+    // new users inherit the owner's changes while a non-owner's changes stay in their copy.
+    if (!note.isPersonalizedMode()
+        || authorizationService.isOwner(note.getId(), context.getUserAndRoles())) {
+      p.setText(text);
+      p.setTitle(title);
+      p.setAuthenticationInfo(context.getAutheInfo());
+      if (params != null && !params.isEmpty()) {
+        p.settings.setParams(params);
+      }
+      if (config != null && !config.isEmpty()) {
+        p.mergeConfig(config);
+      }
     }
 
     if (note.isPersonalizedMode()) {
@@ -761,10 +767,15 @@ public void updateParagraph(String noteId,
           callback.onFailure(new ParagraphNotFoundException(paragraphId), context);
           return null;
         }
-        p.settings.setParams(params);
-        p.mergeConfig(config);
-        p.setTitle(title);
-        p.setText(text);
+        // In personalized mode only the note owner may update the master paragraph, so that
+        // new users inherit the owner's changes while a non-owner's changes stay in their copy.
+        if (!note.isPersonalizedMode()
+            || authorizationService.isOwner(noteId, context.getUserAndRoles())) {
+          p.settings.setParams(params);
+          p.mergeConfig(config);
+          p.setTitle(title);
+          p.setText(text);
+        }
         if (note.isPersonalizedMode()) {
           p = p.getUserParagraph(context.getAutheInfo().getUser());
           p.settings.setParams(params);
@@ -1393,16 +1404,21 @@ private Paragraph setParagraphUsingMessage(Note note, Message fromMessage, Strin
                                              String text, String title, Map<String, Object> params,
                                              Map<String, Object> config) {
     Paragraph p = note.getParagraph(paragraphId);
-    p.setText(text);
-    p.setTitle(title);
     AuthenticationInfo subject =
         new AuthenticationInfo(fromMessage.principal, fromMessage.roles, fromMessage.ticket);
-    p.setAuthenticationInfo(subject);
-    p.settings.setParams(params);
-    p.setConfig(config);
+    // In personalized mode only the note owner may update the master paragraph, so that
+    // new users inherit the owner's changes while a non-owner's changes stay in their copy.
+    if (!note.isPersonalizedMode()
+        || authorizationService.isOwner(note.getId(), new HashSet<>(subject.getUsersAndRoles()))) {
+      p.setText(text);
+      p.setTitle(title);
+      p.setAuthenticationInfo(subject);
+      p.settings.setParams(params);
+      p.setConfig(config);
+    }
 
     if (note.isPersonalizedMode()) {
-      p = note.getParagraph(paragraphId);
+      p = p.getUserParagraph(subject.getUser());
       p.setText(text);
       p.setTitle(title);
       p.setAuthenticationInfo(subject);

zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java

@@ -36,6 +36,7 @@
 import java.io.FileWriter;
 import java.io.IOException;
 import java.nio.file.Files;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -88,6 +89,7 @@ class NotebookServiceTest {
   private File confDir;
   private SearchService searchService;
   private Notebook notebook;
+  private AuthorizationService authorizationService;
   private ServiceContext context =
       new ServiceContext(AuthenticationInfo.ANONYMOUS, new HashSet<>());
 
@@ -136,8 +138,7 @@ void setUp(TestInfo testInfo) throws Exception {
     when(mockInterpreterSetting.getStatus()).thenReturn(InterpreterSetting.Status.READY);
     Credentials credentials = new Credentials();
     NoteManager noteManager = new NoteManager(notebookRepo, zConf);
-    AuthorizationService authorizationService =
-        new AuthorizationService(noteManager, zConf, storage);
+    authorizationService = new AuthorizationService(noteManager, zConf, storage);
     notebook =
         new Notebook(
             zConf,
@@ -588,6 +589,88 @@ void testParagraphOperations() throws IOException {
     verify(callback).onSuccess(p, context);
   }
 
+  @Test
+  void testRunParagraphInPersonalizedModeDoesNotPolluteMasterParagraph() throws IOException {
+    String note1Id = notebookService.createNote("/note_personalized", "test", true, context, callback);
+    // make "admin" the note owner so that "user1" below is a non-owner
+    authorizationService.setOwners(note1Id, Collections.singleton("admin"));
+    Map<String, Object> masterParams = new HashMap<>();
+    masterParams.put("name", "master");
+    String paragraphId = notebook.processNote(note1Id,
+      note1 -> {
+        note1.setPersonalizedMode(true);
+        Paragraph p = note1.getParagraph(0);
+        p.setText("1+1");
+        p.settings.setParams(masterParams);
+        return p.getId();
+      });
+
+    ServiceContext user1Context = new ServiceContext(new AuthenticationInfo("user1"),
+        new HashSet<>(Collections.singleton("user1")));
+    Map<String, Object> user1Params = new HashMap<>();
+    user1Params.put("name", "user1");
+
+    reset(callback);
+    boolean runStatus = notebook.processNote(note1Id,
+      note1 -> {
+        return notebookService.runParagraph(note1, paragraphId, "user1_title", "1+1",
+          user1Params, new HashMap<>(), null, false, true, user1Context, callback);
+      });
+    assertTrue(runStatus);
+
+    notebook.processNote(note1Id,
+      note1 -> {
+        Paragraph master = note1.getParagraph(paragraphId);
+        assertEquals(masterParams, master.settings.getParams());
+        assertNull(master.getTitle());
+        Paragraph user1Paragraph = master.getUserParagraph("user1");
+        assertEquals(user1Params, user1Paragraph.settings.getParams());
+        assertEquals("user1_title", user1Paragraph.getTitle());
+        return null;
+      });
+
+    // updateParagraph must not pollute the master paragraph either
+    reset(callback);
+    Map<String, Object> user1UpdatedParams = new HashMap<>();
+    user1UpdatedParams.put("name", "user1_updated");
+    notebookService.updateParagraph(note1Id, paragraphId, "user1_updated_title", "1+1",
+        user1UpdatedParams, new HashMap<>(), user1Context, callback);
+
+    notebook.processNote(note1Id,
+      note1 -> {
+        Paragraph master = note1.getParagraph(paragraphId);
+        assertEquals(masterParams, master.settings.getParams());
+        assertNull(master.getTitle());
+        Paragraph user1Paragraph = master.getUserParagraph("user1");
+        assertEquals(user1UpdatedParams, user1Paragraph.settings.getParams());
+        assertEquals("user1_updated_title", user1Paragraph.getTitle());
+        return null;
+      });
+
+    // the note owner's changes must reach the master paragraph so new users inherit them
+    reset(callback);
+    ServiceContext adminContext = new ServiceContext(new AuthenticationInfo("admin"),
+        new HashSet<>(Collections.singleton("admin")));
+    Map<String, Object> adminParams = new HashMap<>();
+    adminParams.put("name", "admin");
+    notebookService.updateParagraph(note1Id, paragraphId, "admin_title", "1+1",
+        adminParams, new HashMap<>(), adminContext, callback);
+
+    notebook.processNote(note1Id,
+      note1 -> {
+        Paragraph master = note1.getParagraph(paragraphId);
+        assertEquals(adminParams, master.settings.getParams());
+        assertEquals("admin_title", master.getTitle());
+        Paragraph adminParagraph = master.getUserParagraph("admin");
+        assertEquals(adminParams, adminParagraph.settings.getParams());
+        assertEquals("admin_title", adminParagraph.getTitle());
+        // the non-owner's personal copy must keep their own values
+        Paragraph user1Paragraph = master.getUserParagraph("user1");
+        assertEquals(user1UpdatedParams, user1Paragraph.settings.getParams());
+        return null;
+      });
+  }
+
   @Test
   void testNormalizeNotePath() throws IOException {
     assertEquals("/Untitled Note", notebookService.normalizeNotePath(" "));