Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes the logic for Parameter existence to use PSBoundParameters.ContainsKey() (#56) #57

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

TriggerAu
Copy link
Contributor

The issue in the logic here was that parameter existence was using an if($param) check and if the parameter value is $false then the $null check has the same result. By using PSBoundParameters.ContainsKey() we can check if the parameter was passed in the command

This logic works for whether the parameter is named or positional

This PR makes the fix in the mustache and generates the code files

IssueL #56

…ainsKey()

The issue in the logic here was that parameter existence was ising an if($param)
check and if the parameter value is $false then the $null check has the same result.
By using PSBoundParameters.ContainsKey() we can check if the parameter was passed
in the command

This logic works for whether the parameter is named or positional
@laura-rodriguez
Copy link
Collaborator

Hi @TriggerAu,

Thank you for contributing this fix to the Okta PS module! I filed an internal ticket so the team can prioritize accordingly.

Internal Ref: OKTA-824406

@laura-rodriguez
Copy link
Collaborator

Hi @TriggerAu,

I appreciate your patience.

I was reviewing your PR and looking into the original issue you filed (#56). It seems that your proposed solution, while it works for most cases, still doesn't fix the issue you had with New-OktaUser -Body $body -Activate $false.

I've added a test to verify that the underlying function Invoke-OktaApiClient is invoked with the corresponding QueryParams:

It 'Test New-OktaUser with query params' {     
            $Content = '{"id":"00u8zkhk9tWf3aWsq1d7","status":"ACTIVE", .... }' | ConvertFrom-Json #redacted for simplicity
            $Response = @{
                Response   = $Content
                StatusCode = 200
                Headers = @{ "Content-Type" = @("application/json")}
            }

            Mock -ModuleName Okta.PowerShell Invoke-OktaApiClient { return $Response } -Verifiable

            $UserProfile = [PSCustomObject]@{
                firstName = 'John'
                lastName = 'Doe'
                login = '[email protected]'
                email = '[email protected]'
            }

            $CreateUserRequest = Initialize-OktaCreateUserRequest -VarProfile $UserProfile -GroupIds 'foo'
            New-OktaUser -Body $CreateUserRequest -Activate $true -Provider $false -NextLogin "changePassword"

            Assert-MockCalled -ModuleName Okta.PowerShell Invoke-OktaApiClient -Times 1 -Scope It -ParameterFilter {
                $null -ne $QueryParameters -and
                $QueryParameters['activate']  -eq $true -and
                $QueryParameters['provider']  -eq $false -and
                $QueryParameters['nextLogin'] -eq "changePassword"
            }
        }

This test fails when either "Activate" or "Provider" (both of type [System.Nullable[Boolean]]) is false. After debugging, I noticed that $PSBoundParameters.ContainsKey("Activate") only returns true if the value is true and not necessarily when the Activate param is provided. The same happens with the Provider param.

psboundparam-issue

It seems that $PSBoundParameters doesn't work as expected with nullable boolean params. So, I'm curious if you were able to execute NewUser successfully after making these changes.

A potential workaround that seems to be working in my initial tests is replacing $PSBoundParameters.ContainsKey("Activate") with $PSCmdlet.MyInvocation.BoundParameters.ContainsKey("Activate"). This way, we can ensure that nullable params such as Activate or Provider are detected correctly.

Let me know your thoughts.

@TriggerAu
Copy link
Contributor Author

It seems that $PSBoundParameters doesn't work as expected with nullable boolean params. So, I'm curious if you were able to execute NewUser successfully after making these changes.

A potential workaround that seems to be working in my initial tests is replacing $PSBoundParameters.ContainsKey("Activate") with $PSCmdlet.MyInvocation.BoundParameters.ContainsKey("Activate"). This way, we can ensure that nullable params such as Activate or Provider are detected correctly.

Let me know your thoughts.

Hey hey @laura-rodriguez , I struck this just the other day, so not sure if I tested/struck that previously (was a smidge ago), but yes using MyInvocation is the way Im doing a couple of other scripts for switch params so that sounds great

@laura-rodriguez
Copy link
Collaborator

Thank you for your quick response @TriggerAu! FYI I'm working on a fix here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants