RamblingCookieMonster / PSSlack

PowerShell module for simple Slack integration
http://ramblingcookiemonster.github.io/PSSlack/
MIT License
274 stars 74 forks source link

macOS/Linux support #57

Closed devblackops closed 6 years ago

devblackops commented 6 years ago

Greeting Starfighter!

There appear to be a few environment variable related things that prevent PSSlack from working fully on macOS/Linux.

> Set-PSSlackConfig
Export-Clixml : Access to the path '/--PSSlack.xml' is denied.
At /Users/brandon/.local/share/powershell/Modules/PSSlack/0.0.31/Public/Set-PSSlackConfig.ps1:77 char:9
+         Export-Clixml -Path $Path -force
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : OpenError: (:) [Export-Clixml], UnauthorizedAccessException
+ FullyQualifiedErrorId : FileOpenFailure,Microsoft.PowerShell.Commands.ExportClixmlCommand

Specifically, these don't exist in PWSH on macOS: $env:TEMP, $env:USERNAME, $env:COMPUTERNAME

Here is what is available on my machine:

> (Get-ChildItem env:).Name
_
__CF_USER_TEXT_ENCODING
Apple_PubSub_Socket_Render
COLORFGBG
COLORTERM
COMMAND_MODE
HOME
ITERM_PROFILE
ITERM_SESSION_ID
LANG
LOGNAME
PATH
PSModulePath
PWD
SECURITYSESSIONID
SHELL
SHLVL
SSH_AUTH_SOCK
TERM
TERM_PROGRAM
TERM_PROGRAM_VERSION
TERM_SESSION_ID
TMPDIR
USER
XPC_FLAGS
XPC_SERVICE_NAME

I ran into some similar issues with PoshBot when testing on macOS and decided to perform some logic in the PSM1 and come up with my own variables for use by the various functions.

Perhaps PSSlack will need the same and use $env:TMPDIR and $env:USER in place of $env:TEMP and $env:USERNAME. $env:COMPUTERNAME could be replaced with the value from the hostname command.

What do you think?

RamblingCookieMonster commented 6 years ago

Good catch - That works for me!

kanjibates commented 6 years ago

Great to see others interested in PSCore!

Serialization of SecureStrings (Token and Uri in Get- and Set-PSSlackConfig) will also need to be addressed to get PSSlack fully working on macOS/Linux, as that functionality/DPAPI isn't currently supported on non-Windows platforms (PowerShell/PowerShell#1654).

Simplest solution would to be store Token and Uri in plain text on macOS/Linux, and rely on file permissions to limit access.

kanjibates commented 6 years ago

As an aside, PowerShell Core Docker image is missing $env:TEMP/TMPDIR and $env:USER/USERNAME.

PS /> (Get-ChildItem env:).Name                                                                                         
HOME
HOSTNAME
LANG
LC_ALL
PATH
PSModulePath
TERM

It may be worth taking a cue from PSCore here, and use $env:USERPROFILE (although I feel $env:APPDATA might be more appropriate) on Windows and $env:HOME on other platforms.

It may also be worth considering changing the config name to PSSlack.xml (or the more *nix-y .psslack) on non-Windows platforms -- without DPAPI preventing decryption of Tokens and Uris generated on different hosts or by different users, the <user>-<host>- prefix doesn't make much sense.

Thoughts?

RamblingCookieMonster commented 6 years ago

Hiyo!

Good catch - yeah, I'd agree on the naming convention and using a dot file - if we did serialize secrets to disk, I'd probably prefer to require some sort of ack switch (i.e. not default, make it explicitly clear what is happening), e.g. -SaveSecretsToDisk that's only applicable on the *nix side (implementation of that might be odd)

devblackops commented 6 years ago

So how about these two options to calculate the psslack.xml path?

Keep existing config file path for legacy PS versions
if (($PSEdition -eq 'Desktop') -or (Test-Path -Path 'Variable:\IsWindows')) {
    $_PSSlackXmlpath = Join-Path -Path $env:TEMP -ChildPath "$env:USERNAME-$env:COMPUTERNAME-PSSlack.xml"
} else {
    $_PSSlackXmlpath = Join-Path -Path $home -ChildPath '.psslack.xml'
}

or

Use new path for config file across the board
$_PSSlackXmlpath = Join-Path -Path $home -ChildPath '.psslack.xml'

I would lean towards the former to keep the existing behavior for Windows PowerShell unless you're cool with a breaking change.

kanjibates commented 6 years ago

I think the first makes most sense to maintain compatibility, but -- as mentioned above -- think $env:APPDATA/psslack.xml makes a lot more sense.

If someone really needs to the USERNAME-COMPUTERNAME-prefix format because of DPAPI, then that functionality is still available by calling Get-/Set-PSSlackConfig explicitly with the old path.

function Test-IsWindows
{
    [CmdletBinding()]
    [OutputType([bool])]
    param()

    end
    {
        !(Test-Path -Path Variable:\IsWindows) -or $IsWindows
    }
}

function Get-PSSlackConfigPath
{
    [CmdletBinding()]
    param()

    end
    {
        if (Test-IsWindows)
        {
            Join-Path -Path $env:TEMP -ChildPath "$env:USERNAME-$env:COMPUTERNAME-PSSlack.xml"
        }
        else
        {
            Join-Path -Path $env:HOME -ChildPath '.psslack' # Leading . and no file extension to be Unixy.
        }
    }
}

I was contemplating rolling these out as functions for easier testing/mocking/extension/re-use.

Anyway, let me know if you plan on working on this, and I'll be happy to find another issues. Or hit me up in Slack and maybe we can submit a PR together?

devblackops commented 6 years ago

It's all yours @kanjibates 😄

FYI, In Jakul's Configuration module he went with $env:LocalAppData and $home/.config/ for Windows and Linux/macOS respectively to fix a similar issue in commit https://github.com/PoshCode/Configuration/commit/212e38ef13cab2f151a0517f6f13f2c0deb2a8a2