Repository navigation
Add OAuth support to New-PASSession function - #661
johannesconsulting wants to merge 1 commit into
Conversation
pspete
left a comment
There was a problem hiding this comment.
user error @johannesconsulting 😂
I never submitted them
| try { | ||
|
|
||
| #Get Authenticated User. | ||
| $User = Get-PASLoggedOnUser -ErrorAction Stop |
There was a problem hiding this comment.
for every other parameter set, this call happens after a real logon request has already succeeded against the server, so treating its failure as non-fatal is correct because authentication suceeded already.
For OAuth, though, there's no logon request at all, so this is the only point in the OAuth path where the token is ever actually checked against the server. If it fails here for OAuth, should that be fatal rather than falling through to $User = $null with New-PASSession still reporting success?
|
|
||
| if ($PSCmdlet.ParameterSetName -eq 'OAuth' -and -not $SkipVersionCheck) { | ||
|
|
||
| Assert-VersionRequirement -RequiredVersion 15.2 |
There was a problem hiding this comment.
This runs after BaseURI, ApiURI, the live Authorization header, and ExternalVersion (lines 920-946) are already set in $psPASSession.
If the version check fails, the thrown exception is the only visible sign of failure, but $psPASSession is left holding a fully working session against a vault that was just rejected for being too old.
Any subsequent psPAS call would silently use it. Since I don't think this check can easily move earlier, should $psPASSession be rolled back on failure, not left committed?
| 'OAuth' { | ||
|
|
||
| #BaseURI required in module scope for Assert-VersionRequirement | ||
| $psPASSession.BaseURI = $Uri |
There was a problem hiding this comment.
Unlike every other parameter set here, the OAuth branch starts writing to the module-scoped $psPASSession (BaseURI here, then WebSession at line 645) before $PSCmdlet.ShouldProcess (line 742) and outside any try/catch.
This means:
- WhatIf doesn't prevent this session mutation for OAuth.
- If Assert-VersionRequirement -SelfHosted (line 609) throws right after, BaseURI is already overwritten and never restored, corrupting any previously-valid session.
Could this branch build a local variable (matching the other parameter sets, which only populate $LogonRequest at this stage) and defer the $psPASSession writes until after the checks pass?
| } | ||
|
|
||
| Mock Get-PASLoggedOnUser -MockWith { | ||
| @{'UserName' = 'SomeUser' } |
There was a problem hiding this comment.
This mock returns a Hashtable, where every other context in this file uses [PSCustomObject].
could this be changed to [PSCustomObject]@{ UserName = 'SomeUser' } to match the rest of the suite?
Description
OAuthparameter set toNew-PASSession, allowing authentication using a SecureStringAccessTokeninstead of username/password or other methods. The function now sets the required HTTP headers for OAuth and handles certificate options. The certificate options are somewhat unneeded but can maybe needed when load balancer/proxy require mTLS or something...CyberArks implementation of OAuth is not really like SAML or other auth methods. It is not the one triggering the login flow. It already expects that flow to be done. It just wants the token that you have. CyberArks job is just to validate it.
So the New-PASSession is a little bit different then other auth methods.
Type of change
How Has This Been Tested?
Test Configuration:
Checklist: