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

Add Published and PublishedOn attributes to template objects #191

Merged
merged 8 commits into from
Dec 11, 2024

Conversation

jakehildreth
Copy link
Owner

To properly assess the risk presented by a single finding, Locksmith must know if the template is available for enrollment on one or more CAs. This is support of #117.

Copy link
Collaborator

@SamErde SamErde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No show-stoppers! Just note the comment about supporting values from the pipeline.


[CmdletBinding(SupportsShouldProcess)]
param (
[parameter(Mandatory, ValueFromPipeline)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accepting values from the pipeline requires a process {} block. The begin and end blocks are optional, but process is required to bind to the pipeline object.

Set-AdditionalTemplateProperty -ADCSObjects $ADCSObjects -ForestGC 'dc1.ad.dotdot.horse:3268'
#>

[CmdletBinding(SupportsShouldProcess)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function should ideally either suppress the PSScriptAnalyzer check for SupportsShouldProcess or actually wrap the set/add action in a ShouldProcess block. Because (IIRC) this is modifying a transient object within the script and not a system object, you're probably good to just suppress the check and not build in support for ShouldProcess in this function.

@jakehildreth jakehildreth merged commit f6009ed into main Dec 11, 2024
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