Skip to content

Commit 32d4b18

Browse files
ReamerReamer
committed
Fix world readable, writeable, runnable, ownership
1 parent 1388dcd commit 32d4b18

File tree

2 files changed

+80
-38
lines changed

2 files changed

+80
-38
lines changed

‎zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/AuthorizationService.java‎

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -244,50 +244,40 @@ public Set<String> getRoles(String user) {
244244
}
245245

246246
public boolean isOwner(String noteId, Set<String> entities) {
247-
Set<String> owners = new HashSet<>(getOwners(noteId));
248-
owners.addAll(getDefaultOwners());
249-
return isMember(entities, owners) || isAdmin(entities);
247+
return isMember(entities, constructRoles(getOwners(noteId), getDefaultOwners())) ||
248+
isAdmin(entities);
250249
}
251250

252251
public boolean isWriter(String noteId, Set<String> entities) {
253-
Set<String> owners = new HashSet<>(getOwners(noteId));
254-
owners.addAll(getDefaultOwners());
255-
Set<String> writers = new HashSet<>(getWriters(noteId));
256-
writers.addAll(getDefaultWriters());
257-
return isMember(entities, writers) ||
258-
isMember(entities, owners) ||
252+
return isMember(entities, constructRoles(getWriters(noteId), getDefaultWriters())) ||
253+
isMember(entities, constructRoles(getOwners(noteId), getDefaultOwners())) ||
259254
isAdmin(entities);
260255
}
261256

262257
public boolean isReader(String noteId, Set<String> entities) {
263-
Set<String> owners = new HashSet<>(getOwners(noteId));
264-
owners.addAll(getDefaultOwners());
265-
Set<String> writers = new HashSet<>(getWriters(noteId));
266-
writers.addAll(getDefaultWriters());
267-
Set<String> runners = new HashSet<>(getRunners(noteId));
268-
runners.addAll(getDefaultRunners());
269-
Set<String> readers = new HashSet<>(getReaders(noteId));
270-
readers.addAll(getDefaultReaders());
271-
return isMember(entities, readers) ||
272-
isMember(entities, owners) ||
273-
isMember(entities, writers) ||
274-
isMember(entities, runners) ||
258+
return isMember(entities, constructRoles(getReaders(noteId), getDefaultReaders())) ||
259+
isMember(entities, constructRoles(getOwners(noteId), getDefaultOwners())) ||
260+
isMember(entities, constructRoles(getWriters(noteId), getDefaultWriters())) ||
261+
isMember(entities, constructRoles(getRunners(noteId), getDefaultRunners())) ||
275262
isAdmin(entities);
276263
}
277264

278265
public boolean isRunner(String noteId, Set<String> entities) {
279-
Set<String> owners = new HashSet<>(getOwners(noteId));
280-
owners.addAll(getDefaultOwners());
281-
Set<String> writers = new HashSet<>(getWriters(noteId));
282-
writers.addAll(getDefaultWriters());
283-
Set<String> runners = new HashSet<>(getRunners(noteId));
284-
runners.addAll(getDefaultRunners());
285-
return isMember(entities, runners) ||
286-
isMember(entities, writers) ||
287-
isMember(entities, owners) ||
266+
return isMember(entities, constructRoles(getRunners(noteId), getDefaultRunners())) ||
267+
isMember(entities, constructRoles(getWriters(noteId), getDefaultWriters())) ||
268+
isMember(entities, constructRoles(getOwners(noteId), getDefaultOwners())) ||
288269
isAdmin(entities);
289270
}
290271

272+
private Set<String> constructRoles(Set<String> noteRoles, Set<String> globalRoles) {
273+
Set<String> roles = new HashSet<>(noteRoles);
274+
// If the note has no role, the note right is for everyone, so we are not allowed to add the default roles
275+
if (!roles.isEmpty()) {
276+
roles.addAll(globalRoles);
277+
}
278+
return roles;
279+
}
280+
291281
private Set<String> getDefaultOwners() {
292282
return getDefaultRoles(ZeppelinConfiguration.ConfVars.ZEPPELIN_OWNER_ROLES);
293283
}

‎zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/AuthorizationServiceTest.java‎

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
import java.io.IOException;
2626
import java.util.Arrays;
27+
import java.util.Collections;
2728
import java.util.HashSet;
2829

2930
import org.apache.zeppelin.conf.ZeppelinConfiguration;
@@ -40,6 +41,8 @@ class AuthorizationServiceTest {
4041
private AuthorizationService authorizationService;
4142
private static final String BLANK_ROLE = " ";
4243
private static final String EMPTY_ROLE = "";
44+
private static final String TEST_USER_1 = "TestUser1";
45+
private static final String TEST_USER_2 = "TestUser2";
4346

4447
@BeforeEach
4548
private void setup() throws IOException {
@@ -54,7 +57,7 @@ private void setup() throws IOException {
5457
@Test
5558
void testDefaultOwners() throws IOException {
5659
Note testNote = new Note();
57-
authorizationService.createNoteAuth(testNote.getId(), new AuthenticationInfo("TestUser"));
60+
authorizationService.createNoteAuth(testNote.getId(), new AuthenticationInfo(TEST_USER_1));
5861

5962
// Comma separated with trim
6063
when(zConf.getString(ZeppelinConfiguration.ConfVars.ZEPPELIN_OWNER_ROLES)).thenReturn("TestGroup, TestGroup2");
@@ -73,14 +76,14 @@ void testDefaultOwners() throws IOException {
7376
assertFalse(authorizationService.isOwner(testNote.getId(), new HashSet<>(Arrays.asList(EMPTY_ROLE))));
7477
// Empty - null
7578
when(zConf.getString(ZeppelinConfiguration.ConfVars.ZEPPELIN_OWNER_ROLES)).thenReturn(null);
76-
assertTrue(authorizationService.isOwner(testNote.getId(), new HashSet<>(Arrays.asList("TestUser"))));
79+
assertTrue(authorizationService.isOwner(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_1))));
7780

7881
}
7982

8083
@Test
8184
void testDefaultRunners() throws IOException {
8285
Note testNote = new Note();
83-
authorizationService.createNoteAuth(testNote.getId(), new AuthenticationInfo("TestUser"));
86+
authorizationService.createNoteAuth(testNote.getId(), new AuthenticationInfo(TEST_USER_1));
8487

8588
// Comma separated with trim
8689
when(zConf.getString(ZeppelinConfiguration.ConfVars.ZEPPELIN_RUNNER_ROLES)).thenReturn("TestGroup, TestGroup2");
@@ -99,13 +102,13 @@ void testDefaultRunners() throws IOException {
99102
assertFalse(authorizationService.isRunner(testNote.getId(), new HashSet<>(Arrays.asList(EMPTY_ROLE))));
100103
// Empty - null
101104
when(zConf.getString(ZeppelinConfiguration.ConfVars.ZEPPELIN_RUNNER_ROLES)).thenReturn(null);
102-
assertTrue(authorizationService.isRunner(testNote.getId(), new HashSet<>(Arrays.asList("TestUser"))));
105+
assertTrue(authorizationService.isRunner(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_1))));
103106
}
104107

105108
@Test
106109
void testDefaultWriters() throws IOException {
107110
Note testNote = new Note();
108-
authorizationService.createNoteAuth(testNote.getId(), new AuthenticationInfo("TestUser"));
111+
authorizationService.createNoteAuth(testNote.getId(), new AuthenticationInfo(TEST_USER_1));
109112

110113
// Comma separated with trim
111114
when(zConf.getString(ZeppelinConfiguration.ConfVars.ZEPPELIN_WRITER_ROLES)).thenReturn("TestGroup, TestGroup2");
@@ -124,13 +127,13 @@ void testDefaultWriters() throws IOException {
124127
assertFalse(authorizationService.isWriter(testNote.getId(), new HashSet<>(Arrays.asList(EMPTY_ROLE))));
125128
// Empty - null
126129
when(zConf.getString(ZeppelinConfiguration.ConfVars.ZEPPELIN_WRITER_ROLES)).thenReturn(null);
127-
assertTrue(authorizationService.isWriter(testNote.getId(), new HashSet<>(Arrays.asList("TestUser"))));
130+
assertTrue(authorizationService.isWriter(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_1))));
128131
}
129132

130133
@Test
131134
void testDefaultReaders() throws IOException {
132135
Note testNote = new Note();
133-
authorizationService.createNoteAuth(testNote.getId(), new AuthenticationInfo("TestUser"));
136+
authorizationService.createNoteAuth(testNote.getId(), new AuthenticationInfo(TEST_USER_1));
134137

135138
// Comma separated with trim
136139
when(zConf.getString(ZeppelinConfiguration.ConfVars.ZEPPELIN_READER_ROLES)).thenReturn("TestGroup, TestGroup2");
@@ -149,6 +152,55 @@ void testDefaultReaders() throws IOException {
149152
assertFalse(authorizationService.isReader(testNote.getId(), new HashSet<>(Arrays.asList(EMPTY_ROLE))));
150153
// Empty - null
151154
when(zConf.getString(ZeppelinConfiguration.ConfVars.ZEPPELIN_READER_ROLES)).thenReturn(null);
152-
assertTrue(authorizationService.isReader(testNote.getId(), new HashSet<>(Arrays.asList("TestUser"))));
155+
assertTrue(authorizationService.isReader(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_1))));
153156
}
157+
158+
@Test
159+
void testWorldReadable() throws IOException {
160+
Note testNote = new Note();
161+
authorizationService.createNoteAuth(testNote.getId(), new AuthenticationInfo(TEST_USER_1));
162+
authorizationService.setReaders(testNote.getId(), Collections.emptySet());
163+
164+
assertTrue(authorizationService.isReader(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
165+
assertFalse(authorizationService.isRunner(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
166+
assertFalse(authorizationService.isWriter(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
167+
assertFalse(authorizationService.isOwner(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
168+
}
169+
170+
@Test
171+
void testWorldRunnable() throws IOException {
172+
Note testNote = new Note();
173+
authorizationService.createNoteAuth(testNote.getId(), new AuthenticationInfo(TEST_USER_1));
174+
authorizationService.setRunners(testNote.getId(), Collections.emptySet());
175+
176+
assertTrue(authorizationService.isReader(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
177+
assertTrue(authorizationService.isRunner(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
178+
assertFalse(authorizationService.isWriter(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
179+
assertFalse(authorizationService.isOwner(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
180+
}
181+
182+
@Test
183+
void testWorldWritable() throws IOException {
184+
Note testNote = new Note();
185+
authorizationService.createNoteAuth(testNote.getId(), new AuthenticationInfo(TEST_USER_1));
186+
authorizationService.setWriters(testNote.getId(), Collections.emptySet());
187+
188+
assertTrue(authorizationService.isReader(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
189+
assertTrue(authorizationService.isRunner(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
190+
assertTrue(authorizationService.isWriter(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
191+
assertFalse(authorizationService.isOwner(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
192+
}
193+
194+
@Test
195+
void testWorldOwnership() throws IOException {
196+
Note testNote = new Note();
197+
authorizationService.createNoteAuth(testNote.getId(), new AuthenticationInfo(TEST_USER_1));
198+
authorizationService.setOwners(testNote.getId(), Collections.emptySet());
199+
200+
assertTrue(authorizationService.isReader(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
201+
assertTrue(authorizationService.isRunner(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
202+
assertTrue(authorizationService.isWriter(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
203+
assertTrue(authorizationService.isOwner(testNote.getId(), new HashSet<>(Arrays.asList(TEST_USER_2))));
204+
}
205+
154206
}

0 commit comments

Comments
 (0)