Set-RabbitMqConfig Access Denied for Non-Elevated User Fix

[PowershellNinja]

Hi!

I was using PSRabbitMq, specifically Set-RabbitMqConfig, when I ran into an access denied error. I was using Powershell as a non-elevated user which effectively denied me access to C:\Program Files\WindowsPowershell\Modules\PsRabbitMQ\RabbitMqConfig.xml.

To fix this I added a "Persist" Switch to the Set-RabbitMqConfig function which causes the current configuration also the be written into the file (in addition to writing it to the variable) and warns the user if there happens an error while writing the config. I also added some help text for the new parameter and a Tests file for the whole function.

Initially I wanted the catch block to only catch exceptions of the type System.UnauthorizedAccessException, which worked fine, but Pester doesn't seem to respect the Exception Type I specify and just generates a generic Exception, so I had to remove the catch error type statement. If someone knows how to work around this, I would be glad if you could share your knowledge ;-)

Regards, Raphael

[RamblingCookieMonster]

Makes sense to me!

To keep consistent with existing behavior (although, agreed, would go with switch if this wasn't already a thing), it might be worth switching to a boolean that defaults to $true?

Anything I'm missing @gpduck @gaelcolas?

Cheers!

[gpduck]

First off, thanks for submitting tests with the PR!

How do we feel about moving this file out to ProgramData or AppData? As it is right now doesn't it get lost every time you update the module with PowerShellGet (in PSv5)? Maybe we should add a -scope AllUsers/CurrentUser like Install-Module?

As far as the parameter type, I always expect the switch syntax and get annoyed when I run into booleans so my vote would be switch defaulted to $true.

[PowershellNinja]

I think moving it to AppData or ProgramData would solve the issue, but on the other hand would generate a file that gets left behind when deleting the module. But the big plus would be that any user could write the config there.

In regard of the switch/boolean, I myself am more a switch friend. But if the default behaviour should be writing the config to file, maybe a "NoPersist" switch would make more sense to disable the writing to file if needed.

[RamblingCookieMonster]

Agreed on moving to AppData or ProgramData - No preference, can see benefits to both. Given that most users (presumably) don't use RabbitMQ ad hoc, a shared ProgramData config might be handy, but... that introduces access issues for non-admins.

Agreed on NoPersist as well, that works for me!

[PowershellNinja]

I changed the functions (and the .psm1) to use appdata (to avoid non-elevated permission issues). I also changed the -Persist switch parameter on Set-RabbitMQConfig no a -NoPersist switch parameter.

(And I added a test file for Get-PSRabbitMQConfig)

Works for you like this?

[RamblingCookieMonster]

Looks good to me! At some point it might be handy to have a scope of sorts that @gpduck mentioned (e.g. -Scope User would toss it where it is now, -Scope System would use something like $ENV:ProgramData), and with module load and Get without scope using specificity to determine priority (i.e. user takes priority over system)

I could go either way, @gpduck or @gaelcolas might have a preference on whether they'd like this before the merge or later on

Cheers!

[PowershellNinja]

I just added the Scope Parameter to Set-RabbitMQConfig anyway and adjusted Get-RabbitMQConfig and the tests accordingly.

[RamblingCookieMonster]

Awesome, thanks, looks good to me! @gpduck or @gaelcolas any concerns before merging?

Cheers!

[gaelcolas]

Much happier now with the scope :)

Thanks @PowershellNinja for the work! Happy to get this merged!

[gpduck]

Looks good to me, merged. Thanks @PowershellNinja

[PowershellNinja]

Awesome! My first merged contribution to a project, feels really good :)

[RamblingCookieMonster]

Sweet! Not all PRs have as much back and forth; thanks for putting up with and actually implementing the suggestions, even if they were somewhat tangential : D