This is an automated email from the ASF dual-hosted git repository.
tbonelee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 1f80e7a767 [ZEPPELIN-6556] Personalized mode leaks a non-owner's
paragraph edits into the shared master paragraph
1f80e7a767 is described below
commit 1f80e7a7678cb3d580a49c0637ade7ecc04c9d3c
Author: Lee SuJung <[email protected]>
AuthorDate: Mon Aug 10 23:58:56 2026 +0900
[ZEPPELIN-6556] Personalized mode leaks a non-owner's paragraph edits into
the shared master paragraph
### What is this PR for?
A note can be switched to personalized mode so that each user gets their
own copy of a paragraph and one user's form values and results do not affect
another's.
However, `NotebookService.runParagraph` writes the caller's `params`,
`text`, `title` and `config` into the shared master paragraph *before* it
checks whether the note is personalized:
```java
p.setText(text);
p.setTitle(title);
p.setAuthenticationInfo(context.getAutheInfo());
if (params != null && !params.isEmpty()) {
p.settings.setParams(params); // master paragraph
}
if (config != null && !config.isEmpty()) {
p.mergeConfig(config); // master paragraph
}
if (note.isPersonalizedMode()) {
p = p.getUserParagraph(context.getAutheInfo().getUser());
... // the user copy gets the same values
}
```
`notebook.saveNote(...)` then persists the polluted master. The same
ordering exists in `updateParagraph`, and `setParagraphUsingMessage` is worse:
its personalized branch re-fetches the master via
`note.getParagraph(paragraphId)` instead of resolving the user copy, so it
writes the same values into the master twice and never touches the user copy at
all.
So in personalized mode, a non-owner who edits a dynamic form and runs the
paragraph silently overwrites the shared original.
Because `Paragraph.userParagraphMap` is transient, the per-user copies do
not survive a restart. The corruption stays invisible while the copies exist
and surfaces later:
- after a server restart the user copies are gone and every user sees
whatever the last runner wrote
- turning personalized mode off (`Note.clearUserParagraphs`) exposes the
polluted master
- a user opening the note for the first time clones the polluted master
**The fix**: resolve the target paragraph *first* — when the note is
personalized, switch to `getUserParagraph(user)` before any write — so the
master paragraph is never mutated by another user's run or update. Applied to
`runParagraph`, `updateParagraph` and `setParagraphUsingMessage`. Since the two
branches wrote identical values, this also removes the duplicated write blocks.
The only caller of `setParagraphUsingMessage` is `spell()`, which now
records the spell result on the user copy in personalized mode — the intended
behavior — instead of on the shared master.
Note: `PersonalizeActionsIT.testDynamicFormAction` asserts the correct
behavior (a non-owner's edit must not leak) but was passing against the old
server behavior only because a late WebSocket broadcast reverted the typed form
value before the run.
### What type of PR is it?
Bug Fix
### Todos
* [x] Resolve the user paragraph before writing params/text/title/config in
`runParagraph`, `updateParagraph` and `setParagraphUsingMessage`
* [x] Fix `setParagraphUsingMessage` re-fetching the master in its
personalized branch
* [x] Add a unit test that asserts master paragraph integrity in
personalized mode
### What is the Jira issue?
* [ZEPPELIN-6556](https://issues.apache.org/jira/browse/ZEPPELIN-6556)
### How should this be tested?
New test
`NotebookServiceTest#testRunParagraphInPersonalizedModeDoesNotPolluteMasterParagraph`:
on a personalized note, it runs and then updates a paragraph as `user1` with
new params/title, and asserts that the master paragraph's params and title are
unchanged while `getUserParagraph("user1")` picks up the new values.
```
./mvnw -pl zeppelin-server test -Dtest=NotebookServiceTest
-DfailIfNoTests=false
```
Result with the fix: `Tests run: 6, Failures: 0, Errors: 0`.
Manual verification: create a note with `%md echo "hello,
${name=original}"`, run it, enable personalized mode, log in as a second user,
change the form value and run. Then turn personalized mode off (or restart the
server): the paragraph must still show `original`.
### Screenshots (if appropriate)
N/A
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
Closes #5360 from xhaktm00/ZEPPELIN-6556.
Signed-off-by: ChanHo Lee <[email protected]>
---
.../apache/zeppelin/service/NotebookService.java | 52 ++++++++-----
.../zeppelin/service/NotebookServiceTest.java | 87 +++++++++++++++++++++-
2 files changed, 119 insertions(+), 20 deletions(-)
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 554b85f4de..38ce280ad6 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.time.format.DateTimeFormatter;
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 class NotebookService {
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 class NotebookService {
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 @@ public class NotebookService {
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);
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 0a176ac8b4..2d53f34dec 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.File;
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 @@ class NotebookServiceTest {
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 @@ class NotebookServiceTest {
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(" "));
