jongyoul commented on code in PR #4631:
URL: https://github.com/apache/zeppelin/pull/4631#discussion_r1258447761
##########
zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java:
##########
@@ -871,17 +871,60 @@ public void updateNote(String noteId,
return null;
}
- if (!(Boolean) note.getConfig().get("isZeppelinNotebookCronEnable")) {
+ if (!zConf.isZeppelinNotebookCronEnable()) {
+ boolean hasCronSettings = false;
if (config.get("cron") != null) {
- config.remove("cron");
+ LOGGER.warn("cron should be null when cron is disabled");
+ hasCronSettings = true;
+ }
+ if (config.get("cronExecutingUser") != null) {
+ LOGGER.warn("cronExecutingUser should be null when cron is
disabled");
+ hasCronSettings = true;
+ }
+ if (config.get("cronExecutingRoles") != null) {
+ LOGGER.warn("cronExecutingRoles should be null when cron is
disabled");
+ hasCronSettings = true;
+ }
+ if (hasCronSettings) {
+ callback.onFailure(new IllegalArgumentException("Wrong configs"),
context);
+ return null;
+ }
+ } else {
+ String requestCronUser = (String) config.get("cronExecutingUser");
+ List<String> requestCronRoles = (List<String>)
config.get("cronExecutingRoles");
+
+ if
(!authorizationService.hasRunPermission(Collections.singleton(requestCronUser),
note.getId())) {
+ LOGGER.error("Wrong cronExecutingUser: {}", requestCronUser);
+ callback.onFailure(new IllegalArgumentException(requestCronUser),
context);
+ return null;
+ } else {
+ // This part should be restarted but we need to prepare to notice
who can be a cron user in advance
+ if (!context.getUserAndRoles().contains(requestCronUser)) {
+ LOGGER.error("Wrong cronExecutingUser: {}", requestCronUser);
+ callback.onFailure(new
IllegalArgumentException(requestCronUser), context);
+ return null;
+ }
+
+ if (context.getUserAndRoles().containsAll(requestCronRoles)) {
Review Comment:
There was a report for illegal access and calls that the attackers can set
corn-related things freely without any validation logic. This is a brute-force
way to minimize the possibility. We should prepare the way for who can be a
cron user for notes but before preparing the way, I think it should be set only
by those who have run permissions and the roles.
--
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: reviews-unsubscr...@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
