jongyoul commented on code in PR #4624:
URL: https://github.com/apache/zeppelin/pull/4624#discussion_r1244500227
##########
zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java:
##########
@@ -1497,4 +1530,78 @@ private <T> boolean checkPermission(String noteId,
return false;
}
}
+
+ /**
+ * Return null when it is allowed, otherwise return the error message which
could be
+ * propagated to frontend
+ *
+ * @param noteInfo
+ * @param context
+ * @param permission
+ * @param op
+ * @return
+ */
+ private <T> boolean checkPermission(NoteInfo noteInfo,
+ Permission permission,
+ Message.OP op,
+ ServiceContext context,
+ ServiceCallback<T> callback) throws
IOException {
+ boolean isAllowed = false;
+ Set<String> allowed = null;
+ switch (permission) {
+ case READER:
+ isAllowed = authorizationService.isReader(noteInfo.getId(),
context.getUserAndRoles());
+ allowed = authorizationService.getReaders(noteInfo.getId());
+ break;
+ case WRITER:
+ isAllowed = authorizationService.isWriter(noteInfo.getId(),
context.getUserAndRoles());
+ allowed = authorizationService.getWriters(noteInfo.getId());
+ break;
+ case RUNNER:
+ isAllowed = authorizationService.isRunner(noteInfo.getId(),
context.getUserAndRoles());
+ allowed = authorizationService.getRunners(noteInfo.getId());
+ break;
+ case OWNER:
+ isAllowed = authorizationService.isOwner(noteInfo.getId(),
context.getUserAndRoles());
+ allowed = authorizationService.getOwners(noteInfo.getId());
+ break;
+ }
+ if (isAllowed) {
+ return true;
+ } else {
+ String errorMsg = String.format(
+ "Insufficient %s privileges to '%s' note.\n" +
+ "Allowed users or roles: %s\n" +
+ "But the user %s belongs to: %s",
+ permission, noteInfo.getNoteName(),
+ allowed,
+ context.getAutheInfo().getUser(), context.getUserAndRoles());
+ callback.onFailure(new ForbiddenException(errorMsg), context);
Review Comment:
Oh, I see this code but we'd better move this code outside of this method
because it's only adopted when failure. I think it would be good to make it
clear the purpose of this method and `callbak.onFailure` doesn't seem to be the
purpose of this method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
