Skip to content

[ZEPPELIN-3350] Don't allow set cronExecutionUser#2914

Closed
zjffdu wants to merge 1 commit into
apache:masterfrom
zjffdu:ZEPPELIN-3350
Closed

[ZEPPELIN-3350] Don't allow set cronExecutionUser#2914
zjffdu wants to merge 1 commit into
apache:masterfrom
zjffdu:ZEPPELIN-3350

Conversation

@zjffdu
Copy link
Copy Markdown
Contributor

@zjffdu zjffdu commented Apr 9, 2018

What is this PR for?

This PR just does a quick fix this security issue.

  1. Remove the setting cron user in frontend
  2. Run the note via owner.

What type of PR is it?

[Bug Fix ]

Todos

  • - Task

What is the Jira issue?

How should this be tested?

  • Manually tested

Screenshots (if appropriate)

screen shot 2018-04-09 at 3 04 06 pm

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@zjffdu
Copy link
Copy Markdown
Contributor Author

zjffdu commented Apr 9, 2018

@prabhjyotsingh @felixcheung Could you help review this ? Thanks

Copy link
Copy Markdown
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it was there before, should we have some UI or documentation on why it is gone?


/**
* Run all paragraphs sequentially.
* Run all paragraphs sequentially. Only used for CronJob
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it hard to enforce that only CronJob is calling this?
does it matter - maybe not, now that we are not impersonating any user

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently it is only called by cron. I plan to improve it in 0.9, this is only quick bug fix for 0.8 release.

@zjffdu
Copy link
Copy Markdown
Contributor Author

zjffdu commented Apr 9, 2018

Doc is added

asfgit pushed a commit that referenced this pull request Apr 10, 2018
### What is this PR for?
This PR just does a quick fix this security issue.
1. Remove the setting cron user in frontend
2. Run the note via owner.

### What type of PR is it?
[Bug Fix ]

### Todos
* [ ] - Task

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-3350

### How should this be tested?
* Manually tested

### Screenshots (if appropriate)
![screen shot 2018-04-09 at 3 04 06 pm](https://user-images.githubusercontent.com/164491/38483974-707dca56-3c07-11e8-918a-cd47ed94ee99.png)

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Jeff Zhang <zjffdu@apache.org>

Closes #2914 from zjffdu/ZEPPELIN-3350 and squashes the following commits:

63325d0 [Jeff Zhang] [ZEPPELIN-3350] Don't allow set cronExecutionUser

(cherry picked from commit 08c9ad9)
Signed-off-by: Jeff Zhang <zjffdu@apache.org>
@asfgit asfgit closed this in 08c9ad9 Apr 10, 2018
@weand
Copy link
Copy Markdown
Contributor

weand commented Apr 11, 2018

@zjffdu are you sure this works when logged out? i followed the call stack and I'm wondering if there would happen NPE if no AuthenticationInfo is present:

org.apache.zeppelin.notebook.Note.runAll(AuthenticationInfo, boolean) : Line 659
org.apache.zeppelin.notebook.Paragraph.execute(boolean) : Line 353
org.apache.zeppelin.notebook.Paragraph.setAuthenticationInfo(AuthenticationInfo) : Line 209 << NPE

@zjffdu
Copy link
Copy Markdown
Contributor Author

zjffdu commented Apr 12, 2018

@weand Do you see any issue when logged out ?

@weand
Copy link
Copy Markdown
Contributor

weand commented Apr 12, 2018

not directly when logged out. instead the NPE happens when Zeppelin gets restartet and next cron runs before the note was run manually, which is a frequent use case IMHO.

ERROR [2018-04-12 21:11:00,024] ({DefaultQuartzScheduler_Worker-1} QuartzScheduler.java[schedulerError]:2425) - Job (note.2DBFZ9GGT threw an exception.
org.quartz.SchedulerException: Job threw an unhandled exception. [See nested exception: java.lang.NullPointerException: while trying to invoke the method org.apache.zeppelin.user.AuthenticationInfo.getUser() of a null object loaded from local variable 'authenticationInfo']
at org.quartz.core.JobRunShell.run(JobRunShell.java:213)
at org.quartz.simpl.SimpleThreadPool$WorkerThread.run(SimpleThreadPool.java:573)
Caused by: java.lang.NullPointerException: while trying to invoke the method org.apache.zeppelin.user.AuthenticationInfo.getUser() of a null object loaded from local variable 'authenticationInfo'
at org.apache.zeppelin.notebook.Paragraph.setAuthenticationInfo(Paragraph.java:209)
at org.apache.zeppelin.notebook.Paragraph.execute(Paragraph.java:353)
at org.apache.zeppelin.notebook.Note.run(Note.java:678)
at org.apache.zeppelin.notebook.Note.runAll(Note.java:659)
at org.apache.zeppelin.notebook.Note.runAll(Note.java:648)
at org.apache.zeppelin.notebook.Notebook$CronJob.execute(Notebook.java:936)
at org.quartz.core.JobRunShell.run(JobRunShell.java:202)
... 1 more

@zjffdu
Copy link
Copy Markdown
Contributor Author

zjffdu commented Apr 13, 2018

Thanks @weand I have created ZEPPELIN-3404 for it

1 similar comment
@zjffdu
Copy link
Copy Markdown
Contributor Author

zjffdu commented Apr 13, 2018

Thanks @weand I have created ZEPPELIN-3404 for it

ReedOei pushed a commit to ReedOei/zeppelin that referenced this pull request Apr 16, 2018
### What is this PR for?
This PR just does a quick fix this security issue.
1. Remove the setting cron user in frontend
2. Run the note via owner.

### What type of PR is it?
[Bug Fix ]

### Todos
* [ ] - Task

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-3350

### How should this be tested?
* Manually tested

### Screenshots (if appropriate)
![screen shot 2018-04-09 at 3 04 06 pm](https://user-images.githubusercontent.com/164491/38483974-707dca56-3c07-11e8-918a-cd47ed94ee99.png)

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Jeff Zhang <zjffdu@apache.org>

Closes apache#2914 from zjffdu/ZEPPELIN-3350 and squashes the following commits:

63325d0 [Jeff Zhang] [ZEPPELIN-3350] Don't allow set cronExecutionUser
jwagun pushed a commit to jwagun/zeppelin that referenced this pull request Apr 23, 2018
### What is this PR for?
This PR just does a quick fix this security issue.
1. Remove the setting cron user in frontend
2. Run the note via owner.

### What type of PR is it?
[Bug Fix ]

### Todos
* [ ] - Task

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-3350

### How should this be tested?
* Manually tested

### Screenshots (if appropriate)
![screen shot 2018-04-09 at 3 04 06 pm](https://user-images.githubusercontent.com/164491/38483974-707dca56-3c07-11e8-918a-cd47ed94ee99.png)

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Jeff Zhang <zjffdu@apache.org>

Closes apache#2914 from zjffdu/ZEPPELIN-3350 and squashes the following commits:

63325d0 [Jeff Zhang] [ZEPPELIN-3350] Don't allow set cronExecutionUser
asfgit pushed a commit that referenced this pull request Apr 26, 2018
…before cronjob

### What is this PR for?

This bug is introduced by #2914, this PR will set authenticationInfo using its user as we will store user into note.json.

### What type of PR is it?
[Bug Fix]

### Todos
* [ ] - Task

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-3404

### How should this be tested?
* Manually tested

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Jeff Zhang <zjffdu@apache.org>

Closes #2925 from zjffdu/ZEPPELIN-3404 and squashes the following commits:

b94ecc9 [Jeff Zhang] ZEPPELIN-3404. Fail to run cronjob when user doesn't run it manually before cronjob

(cherry picked from commit 1cea92c)
Signed-off-by: Jeff Zhang <zjffdu@apache.org>
asfgit pushed a commit that referenced this pull request Apr 26, 2018
…before cronjob

### What is this PR for?

This bug is introduced by #2914, this PR will set authenticationInfo using its user as we will store user into note.json.

### What type of PR is it?
[Bug Fix]

### Todos
* [ ] - Task

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-3404

### How should this be tested?
* Manually tested

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Jeff Zhang <zjffdu@apache.org>

Closes #2925 from zjffdu/ZEPPELIN-3404 and squashes the following commits:

b94ecc9 [Jeff Zhang] ZEPPELIN-3404. Fail to run cronjob when user doesn't run it manually before cronjob
prabhjyotsingh pushed a commit to prabhjyotsingh/zeppelin that referenced this pull request Jul 4, 2018
This PR just does a quick fix this security issue.
1. Remove the setting cron user in frontend
2. Run the note via owner.

[Bug Fix ]

* [ ] - Task

* https://issues.apache.org/jira/browse/ZEPPELIN-3350

* Manually tested

![screen shot 2018-04-09 at 3 04 06 pm](https://user-images.githubusercontent.com/164491/38483974-707dca56-3c07-11e8-918a-cd47ed94ee99.png)

* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Jeff Zhang <zjffdu@apache.org>

Closes apache#2914 from zjffdu/ZEPPELIN-3350 and squashes the following commits:

63325d0 [Jeff Zhang] [ZEPPELIN-3350] Don't allow set cronExecutionUser

(cherry picked from commit 08c9ad9)
Signed-off-by: Jeff Zhang <zjffdu@apache.org>
(cherry picked from commit 8528261)

Change-Id: I9ba82bfd36a6bb1968e244503be808375c7071bf
prabhjyotsingh pushed a commit to prabhjyotsingh/zeppelin that referenced this pull request Jul 4, 2018
…before cronjob

This bug is introduced by apache#2914, this PR will set authenticationInfo using its user as we will store user into note.json.

[Bug Fix]

* [ ] - Task

* https://issues.apache.org/jira/browse/ZEPPELIN-3404

* Manually tested

* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Jeff Zhang <zjffdu@apache.org>

Closes apache#2925 from zjffdu/ZEPPELIN-3404 and squashes the following commits:

b94ecc9 [Jeff Zhang] ZEPPELIN-3404. Fail to run cronjob when user doesn't run it manually before cronjob

(cherry picked from commit 1cea92c)
Signed-off-by: Jeff Zhang <zjffdu@apache.org>
(cherry picked from commit eb7969b)

Change-Id: I7c747eaefcc2d7234d0cf07aeaeb26b72c26ee43
mckartha pushed a commit to syntechdev/zeppelin that referenced this pull request Aug 9, 2018
…before cronjob

### What is this PR for?

This bug is introduced by apache#2914, this PR will set authenticationInfo using its user as we will store user into note.json.

### What type of PR is it?
[Bug Fix]

### Todos
* [ ] - Task

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-3404

### How should this be tested?
* Manually tested

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Jeff Zhang <zjffdu@apache.org>

Closes apache#2925 from zjffdu/ZEPPELIN-3404 and squashes the following commits:

b94ecc9 [Jeff Zhang] ZEPPELIN-3404. Fail to run cronjob when user doesn't run it manually before cronjob
mckartha pushed a commit to syntechdev/zeppelin that referenced this pull request Aug 9, 2018
…before cronjob

### What is this PR for?

This bug is introduced by apache#2914, this PR will set authenticationInfo using its user as we will store user into note.json.

### What type of PR is it?
[Bug Fix]

### Todos
* [ ] - Task

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-3404

### How should this be tested?
* Manually tested

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Jeff Zhang <zjffdu@apache.org>

Closes apache#2925 from zjffdu/ZEPPELIN-3404 and squashes the following commits:

b94ecc9 [Jeff Zhang] ZEPPELIN-3404. Fail to run cronjob when user doesn't run it manually before cronjob

(cherry picked from commit 1cea92c)
Signed-off-by: Jeff Zhang <zjffdu@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants