Bug 1716415

Summary:	[V2V][UI] Password leak in popover when we cancel migration at initial stage, need better error handling
Product:	Red Hat CloudForms Management Engine	Reporter:	Yadnyawalk Tale <ytale>
Component:	V2V	Assignee:	Daniel Berger <dberger>
Status:	CLOSED CURRENTRELEASE	QA Contact:	Shveta <sshveta>
Severity:	high	Docs Contact:	Red Hat CloudForms Documentation <cloudforms-docs>
Priority:	high
Version:	5.10.5	CC:	bthurber, dberger, fdupont, mturley, simaishi, sshveta
Target Milestone:	GA	Keywords:	TestOnly, ZStream
Target Release:	5.11.0	Flags:	mturley: needinfo-
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:	v2v
Fixed In Version:	5.11.0.10	Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:
Clones:	1720756 (view as bug list)		Environment:
Last Closed:	2019-12-13 15:08:34 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	Bug
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	V2V	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	1720756

Comment 4 Mike Turley 2019-06-05 12:57:37 UTC

(For some reason I can't see the private comments here, it seems there is still something wrong with my permissions, but YTale filled me in)

For each migration task, the UI displays this property under "Status Detail": task.options.progress.states[task.options.progress.current_state].message

Since the UI simply displays whatever error message the backend provides, I think this will need to be a backend fix. Fabien, do you know how we should proceed here? Can the backend filter out passwords when displaying this kind of error?

Comment 5 Fabien Dupont 2019-06-11 10:17:22 UTC

As Mike mentions, the UI only displays what the backend puts in the task options hash. Recently, to enhance error verbosity, Daniel added more message. However, this exposes to much data.
@Daniel, can you please change the error message in app/models/conversion_host.rb (line 160), to not dump the 'conversion_options' variable ? Here, we mainly catch empty string, so we might just check that and tell that 'Result was empty'.

Comment 6 Daniel Berger 2019-06-11 14:38:38 UTC

I think the blank string being returned is really an issue with the MiqSshUtil wrapper. It should be raising an error.

Anyway, I'll update the method to filter out any sensitive data.

Comment 7 Daniel Berger 2019-06-11 14:57:13 UTC

https://github.com/ManageIQ/manageiq/pull/18852

Comment 9 Shveta 2019-07-02 23:05:16 UTC

Fixed .
Verified in  5.11.0.11.20190625161125_392c61e