diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java index 554b85f4de2..38ce280ad6e 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java +++ b/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 params, Map 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); diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java index 0a176ac8b40..2d53f34dec8 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java +++ b/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 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 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 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 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(" "));