Propagate userid through to create a scanning job with current userid

[nimrodshn]

Currently when a user creates a scanning job we require User.current_user to be some value other than nil. But, when scheduling a scan for some period of time and then logging out / session times out User.current_user turns nil which causes the scanning job to break.

This patch fixes the above behavior to fallback to default user system.

Now we propagate userid through MiqSchedule which solves this issue.

Depends on: ManageIQ/manageiq#17264

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1559459

cc: @cben @zeari

[nimrodshn]

@miq-bot add_label bug

[nimrodshn]

@cben @oourfali should this be backported?

[oourfali]

It is targeted to 5.9.2, so yes.

[nimrodshn]

@miq-bot add_label gaprindashvili/yes

[nimrodshn]

raw_scan_job_create is a callback that is called when scheduling the scan. It is only called through scan_job_create and never directly and now with this patch - always with a userid parameter. That is why I had to add the userid here.

[nimrodshn]

Note that scan_action is inserted into MiqQueue in self.queue_scheduled_work [1]) which is the general method for scheduling actions. Thus, we cannot propagate userid through scan_action which then calls obj.scan [2] without braking all other scheduled entity scans.

[1]https://github.com/ManageIQ/manageiq/blob/85754f5598797bf9b8cf7e1301483c3d14210af5/app/models/miq_schedule.rb#L80

[2]https://github.com/ManageIQ/manageiq/blob/85754f5598797bf9b8cf7e1301483c3d14210af5/app/models/miq_schedule.rb#L195

[JPrause]

@nimrodshn can you add reviewer(s) and an assignee to this PR.

[nimrodshn]

@JPrause I can't add reviewer(s) and an assignee (probably) because I'm not part of manageiq-providers-kubernetes org BUT I am a part of the team maintaining this repo so the tagged people above are aware of this PR 😄 but due to vacation in Israel are unable to take a look... 😢

[agrare]

@nimrodshn I'm not a fan of falling back to the admin user since it could bypass policy/rbac checks. What should happen is when the user creates the job we store the userid string instead of calling User.current_user.userid.

[moolitayer]

It seems that a miq_schedule already has an associated user and that the user information is not passed along to the container image(unlike host.rb for example that does pass that parameter). A few pointers:
https://github.com/ManageIQ/manageiq/blob/c9e7f02baccbc2c2ca3713b00b8787c84c5b0fe9/app/models/miq_schedule.rb#L189

https://github.com/ManageIQ/manageiq/blob/master/app/models/mixins/scanning_mixin.rb#L109

[miq-bot]

Checked commit nimrodshn@0d11ffc with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. ⭐

[nimrodshn]

@cben, @moolitayer @agrare I rewrote this with (a lot of help from @cben 👍 ) it now allows scheduled scan to be associated with userid without falling back by propagating the userid through MiqSchedule

[agrare]

👍 looks much better @nimrodshn

[cben]

LGTM
disclaimer: we partially paired on this

[moolitayer]

LGTM 👍
Let's wait and see what becomes of ManageIQ/manageiq#17264

[agrare]

Core PR has been merged

[agrare]

I'm just going to merge this :) I think @cben forgot

[simaishi]

Gaprindashvili backport details:

$ git log -1
commit 23c742ecc10e85039a35a1e59f324b6bef40ed65
Author: Adam Grare <[email protected]>
Date:   Tue Apr 10 13:27:43 2018 -0400

    Merge pull request #244 from nimrodshn/fix_userid_on_session_logout
    
    Propagate userid through to create a scanning job with current userid
    (cherry picked from commit c19e5a4324b0cff088ca42510d0f8864c79f70a0)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1566529