Redo Windows registry-based deployment profile reading. #4148
Conversation
Reading the registry is a bit strange because sub-object names (keys) live in a different namespace from sub-object named values. But we can lump both sets of names together, and then decide, based on the type of the value of a given name in the schema, whether it's a value in the registry to harvest, or a sub-object key to do further matching on. Yeah, it's a bit hard to explain. But this is why there's only one recursive call to `readRegistryUsingSchema`. Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
There was a problem hiding this comment.
@ericpromislow asked for a rush review (i.e. ignoring style and things and only checking functionality), so the blocking issues are:
parseMultiStringwon't do the correct things for non-string input- We need to convert the values for other types, otherwise we'll break at runtime.
/cc @jandubois (for the review quality).
|
|
||
| if (multiSzValue) { | ||
| // Registry value can be a single-string or even a DWORD and parseMultiString will handle it. | ||
| const arrayValue = nativeReg.parseMultiString(multiSzValue as nativeReg.Value); |
There was a problem hiding this comment.
(This cast is unnecessary, multiSzValue is already a nativeReg.Value)
There was a problem hiding this comment.
It was necessary in my IDE and when I ran the code
| } | ||
| try { | ||
| regValue = readRegistryUsingSchema(schemaVal, innerKey); | ||
| if (regValue && (typeof regValue === 'object') && Object.keys(regValue).length === 0) { |
There was a problem hiding this comment.
I don't understand how regValue can be not 'object'. Inside the call, there is only one return from the end of the function, and newObject is always either null or Record<string, any>.
There was a problem hiding this comment.
Moved to issue #4153
| if (typeof schemaVal === 'boolean') { | ||
| if (typeof regValue === 'number') { | ||
| regValue = regValue !== 0; | ||
| } else { | ||
| console.debug(`Deployment Profile expected boolean value for ${ regPath.concat(schemaKey) }`); | ||
| console.debug(`Deployment Profile expected boolean value for key ${ k }`); |
There was a problem hiding this comment.
This leads to error messages like Deployment Profile expected boolean value for key enabled which is pretty useless for troubleshooting.
There was a problem hiding this comment.
That's because I'm not passing the current path into the recursive function, and I can't get it from the open key AFAICT
There was a problem hiding this comment.
Right, hence we should pass the path in… otherwise the error messages are not useful.
There was a problem hiding this comment.
Moved to issue #4153
| } | ||
| } | ||
| } else { | ||
| regValue = nativeReg.queryValue(regKey, k); | ||
| if (typeof schemaVal === 'boolean') { |
There was a problem hiding this comment.
We need more conversions for schemaVal, e.g. if the user provides hello for a number.
There was a problem hiding this comment.
Note that we're doing zero type-checking when reading profiles on macos or linux
There was a problem hiding this comment.
Moved to issue #4153
Fixes #4144
To test this, you might want to create a new multi-string value in the registry.
According to stackoverflow, there are many ways to do this, running the gamut from powershell and VBS to high-grade ATL/C++. But I just added the values in the regedit textbox, one per line, and made sure that there was an empty line after all the values.