Skip to content
This repository has been archived by the owner on Jun 7, 2022. It is now read-only.

Set-TppPermission validatescript regex #126

Closed
DadsVacayShorts opened this issue Jan 7, 2021 · 13 comments
Closed

Set-TppPermission validatescript regex #126

DadsVacayShorts opened this issue Jan 7, 2021 · 13 comments

Comments

@DadsVacayShorts
Copy link

Environment

Operating System: Windows 10
VenafiTppPS version:2.1.1
PowerShell version:
Name                           Value                                                                                                                           
----                           -----                                                                                                                           
PSVersion                      5.1.17763.1490                                                                                                                  
PSEdition                      Desktop                                                                                                                         
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}                                                                                                         
BuildVersion                   10.0.17763.1490                                                                                                                 
CLRVersion                     4.0.30319.42000                                                                                                                 
WSManStackVersion              3.0                                                                                                                             
PSRemotingProtocolVersion      2.3                                                                                                                             
SerializationVersion           1.1.0.1

Steps to reproduce

When using Set-TppPermission in 2.1.1, there appears to be a validation error. with PrefixedUniversalId that is not present when using raw api calls like this
$SetPerms = Invoke-RestMethod -Method POST -Uri "https://$vtp_host/vedsdk/Permissions/object/$GUID/AD/$Addprovname/$IDApproverUUID" -Body $AddPermsJSON -Headers $headers -ContentType 'application/json' -ErrorVariable RestError`

Expected behavior

I'm expecting that I can take a permissions object and set it to a cert guid for a specific prefixeduniversalid

Set-TppPermission -guid '6874ee6d-9efa-4cf2-9037-7db09f94b6df' -PrefixedUniversalId $item2.PrefixedUniversalId -Permission $item2.ExplicitPermissions

Actual behavior

When I use the above code I get a validation error

Set-TppPermission : Cannot validate argument on parameter 'PrefixedUniversalId'. The "
             $_ -match '(AD|LDAP)+\S+:\w{32}$' -or $_ -match 'local:\w{8}-\w{4}-\w{4}-\w{4}-\w{12}$'
          " validation script for the argument with value "AD+Initech Identity Store:281423fa016e854c998b32bb71f45f1f" did not return a result of True. 
Determine why the validation script failed, and then try the command again.
At C:\Users\nobody\Documents\PKI\Venafi\Cert Migration\Certmigrator1.01.ps1:280 char:85
+ ... 7db09f94b6df' -PrefixedUniversalId $item2.PrefixedUniversalId -Permis ...
+                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidData: (:) [Set-TppPermission], ParameterBindingValidationException
    + FullyQualifiedErrorId : ParameterArgumentValidationError,Set-TppPermission

Looking at the set-tpppermission script the validation script has this regex.

$_ -match '(AD|LDAP)+\S+:\w{32}$' -or $_ -match 'local:\w{8}-\w{4}-\w{4}-\w{4}-\w{12}$'``

I think that is failing because the provider name in my case is going to be a combination of white space and alpha and I think the \S expects the value before the : to have only white space but regex is always a tricky beast so don't quote me on it :).

If I update the regex to this to match any character after (AD|Ldap): with a +. (which is a brute force test here)

$_ -match '(AD|LDAP)+.+:\w{32}$' -or $_ -match 'local:\w{8}-\w{4}-\w{4}-\w{4}-\w{12}$'

, then I can run the function and it applies the permissions object correctly and there are no errors.

Hope that helps.

Screenshots

@gdbarron
Copy link
Owner

gdbarron commented Jan 7, 2021

Interesting, I never thought a domain name could have a space in it so wouldn't have expected that to fail. Your fix definitely gets the job done and what I'll implement in lieu of researching what's allowed for AD domain names and limiting to that which probably isn't needed.

@DadsVacayShorts
Copy link
Author

DadsVacayShorts commented Jan 7, 2021

Interesting, I never thought a domain name could have a space in it so wouldn't have expected that to fail. Your fix definitely gets the job done and what I'll implement in lieu of researching what's allowed for AD domain names and limiting to that which probably isn't needed.

The actual domain doesn't have a space in it (the real name has been obfuscated), but the provider name that comes back for prefixeduniversalid is based on a friendly name of "Domain Identity Store" so "Initech Identity Store" is basically what it looks like just replace Initech with a real domain value. So someone gave it that name way back when. We have other friendly names with descriptive titles as well that are required to match as the provider.

@gdbarron
Copy link
Owner

Thanks for this. I forget it's technically the 'provider' and not domain name.

Update is taking a bit longer than expected as I went down a rabbit hole and cleaning up the entire 'Identity' part of the module. I'd be appreciative if you were able to provide any feedback on the code thus far, https://github.com/gdbarron/VenafiTppPS/compare/issue-126. Still have help updates to do, but much of the code is there. Let me know what you think!

@DadsVacayShorts
Copy link
Author

Sounds great, I'll pull an update and check it out and provide feedback

@gdbarron
Copy link
Owner

Thanks! Quick rundown of updates:

  • Fixed this issue 😃
  • I've standardized on IdentityId for the identity across all identity and permission functions
  • Lots of pipeline updates
    • Get-TppPermission now accepts TppObject, eg. from Find-TppObject
    • Set-TppPermission now accepts output from Get-TppPermission for the object and IdentityId so you only need to specify Permission. No need to get guid and identity manually to pass in.
    • Find-TppIdentity output standardized so you can now pipe to permission functions
  • Added Remove-TppPermission, accepts output from Get-TppPermission
  • I've tried to add aliases for existing param names to reduce code changes on your side (hopefully no changes)
  • Get-TppPermission returns additional object and identity info
  • Add Path param to Set-TppPermission in addition to guid
  • Better error handling and messaging through the permission functions

@DadsVacayShorts
Copy link
Author

DadsVacayShorts commented Jan 14, 2021

Ok, took longer to take a look than I expected. The core issue is fixed for sure for my provider with the space. And while I saw new params in the branch , I kept the code to where it was at before to see if I needed to do any updates and if the aliases will work as is.

I can update the code with the new param values, but here's the examples of the errors I get if I keep it "as is".
For my code base, not an issue making the update, which I'll go ahead and do tomorrow-ish.

Get-TPPIdentityAttribute

$certdeets.approver is in prefixeduniversalid format
Code snippet below

Get-TppIdentityAttribute -PrefixedUniversalId $certdeets.Approver

Error

Get-TppIdentityAttribute : Cannot bind argument to parameter 'IdentityId' because it is null.
At C:\Users\nobody\Documents\PKI\Venafi\Cert Migration\Certmigrator1.09.ps1:470 char:66
+ ... -TppIdentityAttribute -PrefixedUniversalId $item3.PrefixedUniversalId
_+ ~~~~~~~~~~~~~~~~~~~~~~~~~~_
+ CategoryInfo : InvalidData: (:) [Get-TppIdentityAttribute], ParameterBindingValidationException
+ FullyQualifiedErrorId : ParameterArgumentValidationErrorNullNotAllowed,Get-TppIdentityAttribute

Get-TppPermission

Code Snippet
Get-TppPermission -Guid $Guid -Attribute Name -Explicit

Error

_Get-TppPermission : A parameter cannot be found that matches parameter name 'Attribute'.
At C:\Users\nobody\Documents\PKI\Venafi\Cert Migration\Certmigrator1.09.ps1:466 char:50

  •     $Certperms=Get-TppPermission -Guid $Guid -Attribute Name -Exp ..._
    

Set-TppPermission

Code Snippet

Set-TppPermission -guid $GUID -PrefixedUniversalid $Approver.PrefixedUniversalId -Permission $PrivateKeyRead -force -ErrorVariable CertError -ErrorAction SilentlyContinue

Error

Set-TppPermission : Cannot validate argument on parameter 'IdentityId'. The argument is null, empty, or an element of the argument collection contains a null value. Supply a collection that does not contain any null values and then try the command again.
At C:\Users\nobody\Documents\PKI\Venafi\Cert Migration\Certmigrator1.09.ps1:436 char:84
+ ... guid $GUID -PrefixedUniversalid $Approver.PrefixedUniversalId -Permis ...
_+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : InvalidData: (:) [Set-TppPermission], ParameterBindingValidationException
+ FullyQualifiedErrorId : ParameterArgumentValidationError,Set-TppPermission

@gdbarron
Copy link
Owner

Thanks for all the feedback. The Attribute param was removed from Get-TppPermission so definitely expected. I'm surprised the other 2 are new errors as I don't think any changes I've made would trigger that. Are the values being provided truly null or is that a bogus error? The aliases are working so that's good.

@DadsVacayShorts
Copy link
Author

The values were not null and they work if I go against the other branch (with my local update for the regex for set-tpppermission). I'll use the new params and test and report back (though might not be til tonight).

@gdbarron
Copy link
Owner

Appreciate any feedback as I'm only able to repro on either branch with null or empty values. Also, was thinking it would be better to keep the attribute param as is and add a warning it will be deprecated so nothing breaks.

@DadsVacayShorts
Copy link
Author

Ok, I do like the richer info that comes back here with identity info on some of the permission functions . Actually might save time here since you can grab more info in one shot. For my purposes, as this is a new script, it's not a big deal to switch it out to new params, but for someone with an existing codebase , I think keeping the various params and deprecating it makes sense.

I was able to incorporate the new param values and got it working same as the previous commit branch.

The only item to note was on Get-TppPermission is that GUID doesn't seem to work anymore. It seemed like it showed up as a param though still.

It seems like it requires path for sure (which isn't really a big deal..)

Get-TppPermission -path $DN works
get-tpppermission -guid $GUID does not...

$GUID equals {02128cf9-f038-4bc5-a275-8ef0aeb057ef}
Error :
PS C:\Users\nobody\Documents\PKI\Venafi\Cert Migration> get-tpppermission -guid $GUID
ConvertTo-TppGuid : Cannot validate argument on parameter 'Path'. '02128cf9-f038-4bc5-a275-8ef0aeb057ef' is not a valid DN path
At C:\Users\nobody\Documents\WindowsPowerShell\Modules\VenafiTppPS\2.1.1\Classes\TppObject.ps1:47 char:25

+ $info = $Path | ConvertTo-TppGuid -IncludeType
_+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~_
+ CategoryInfo : InvalidData: (02128cf9-f038-4bc5-a275-8ef0aeb057ef:String) [ConvertTo-TppGuid], ParameterBindingValidationException
+ FullyQualifiedErrorId : ParameterArgumentValidationError,ConvertTo-TppGuid
Exception setting "Guid": "Cannot convert null to type "System.Guid"."
At C:\Users\nobody\Documents\WindowsPowerShell\Modules\VenafiTppPS\2.1.1\Classes\TppObject.ps1:50 char:9
+ $this.Guid = $info.Guid
_+ ~~~~~~~~~~~~~~~~~~~~~~~_
+ CategoryInfo : NotSpecified: (:) [], SetValueInvocationException
+ FullyQualifiedErrorId : ExceptionWhenSetting

get-tpppermission : Couldn't obtain list of permissions for . "404 Not Found: The remote server returned an error: (404) Not Found. At line:1 char:1

+ get-tpppermission -guid $GUID
_+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~_
+ CategoryInfo : NotSpecified: (:) [Write-Error], WriteErrorException
+ FullyQualifiedErrorId : Microsoft.PowerShell.Commands.WriteErrorException,Get-TppPermission

@gdbarron
Copy link
Owner

Get-TppPermission -Guid $guid typo has been fixed, thanks for pointing this out. I've also added -Attribute back in with a deprecation warning. At this point I think it's just help updates and we'll be good. Appreciate you working through this with me, great team effort! 😃

@DadsVacayShorts
Copy link
Author

Awesome news and glad to help, also glad this module is out there !

@gdbarron
Copy link
Owner

v2.2 released. @DadsVacayShorts, thanks again and let me know of any issues.

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

No branches or pull requests

2 participants