Skip to content

Feat/Management API#18

Merged
cheinzler-gk merged 22 commits into
mainfrom
feat/RevocationAPI
May 27, 2026
Merged

Feat/Management API#18
cheinzler-gk merged 22 commits into
mainfrom
feat/RevocationAPI

Conversation

@cheinzler-gk

Copy link
Copy Markdown
Contributor

Add functionality of SCEPman 3.1 manage API

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds initial support for SCEPman 3.1 “manage API” operations to the PowerShell module, including searching certificates and revoking issued certificates, plus accompanying documentation and tests.

Changes:

  • Introduces Find-SCEPmanCertificate (manage search API) and Revoke-SCEPmanCertificate (manage revocation API).
  • Exposes new cmdlets via the module manifest and adds supporting enums/constants.
  • Adds Pester tests and extends the README with usage examples.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
Tests/Revoke-SCEPmanCertificate.Tests.ps1 Adds Pester coverage for revocation request formatting and multi-serial behavior.
Tests/Find-SCEPmanCertificate.Tests.ps1 Adds Pester coverage for query building, auth header usage, and optional parameters.
SCEPmanClient/SCEPmanClient.psd1 Exports the new public cmdlets from the module manifest.
SCEPmanClient/Public/Revoke-SCEPmanCertificate.ps1 Implements certificate revocation against the manage API.
SCEPmanClient/Public/Find-SCEPmanCertificate.ps1 Implements certificate search against the manage API with filtering/pagination.
SCEPmanClient/Private/x509/constants.ps1 Adds enums used by the new cmdlets (revocation reasons, validity/type filters).
README.md Documents new search and revoke commands with examples.
Comments suppressed due to low confidence (4)

SCEPmanClient/Public/Find-SCEPmanCertificate.ps1:160

  • Write-Error is emitted and then the code immediately throws a terminating error. This tends to produce duplicate error records for callers. Prefer either throwing a single terminating error (optionally with -ErrorAction Stop semantics) or returning a structured error result without also calling Write-Error.

            Write-Error "$($MyInvocation.MyCommand): Failed to search certificates. Status code: $StatusCode. ApiErrorCode: $ApiErrorCode. ApiErrorMessage: $ApiErrorMessage"

SCEPmanClient/Public/Revoke-SCEPmanCertificate.ps1:135

  • In the catch block, $_.Exception.Response can be $null for non-HTTP failures (DNS/TLS/connection), so accessing .StatusCode can throw and mask the real error. Consider null-checking the Response (and/or falling back to $_.Exception.Status / $_.Exception.Message) before reading StatusCode.
            } catch {
                $statusCode = [int]$_.Exception.Response.StatusCode
                $errorBody = $_.ErrorDetails.Message

SCEPmanClient/Public/Revoke-SCEPmanCertificate.ps1:163

  • On success the cmdlet writes a formatted string via Write-Output, which makes the pipeline output non-object and inconsistent with the other cmdlets in this module (which return objects and use Write-Verbose for status). Consider returning the API response or a structured result object, and reserve human-readable messages for verbose/information streams.
            If ($Result.Success) {
                Write-Output "$($MyInvocation.MyCommand): Certificate $Serial revoked successfully."
            } Else {
                throw "$($MyInvocation.MyCommand): Failed to revoke certificate $Serial. StatusCode: $($Result.StatusCode), ErrorCode: $($Result.ErrorCode), Message: $($Result.ErrorMessage)"
            }

SCEPmanClient/Public/Find-SCEPmanCertificate.ps1:170

  • The expression $ApiErrorMessage ? $ApiErrorMessage : '...' uses the PowerShell ternary operator, which is not supported in Windows PowerShell 5.1. Since the module appears to support PS 5.x (e.g., other code checks $PSVersionTable.PSVersion.Major -lt 7), this should be rewritten using an if/else expression to keep compatibility.
                        404 { throw "$($MyInvocation.MyCommand): Endpoint not found. Verify the URL and that the manage API endpoint exists." }
                        409 { throw "$($MyInvocation.MyCommand): Conflict. $($ApiErrorMessage ? $ApiErrorMessage : 'Request could not be completed.')" }
                        500 { throw "$($MyInvocation.MyCommand): Server error while searching certificates. $ApiErrorMessage" }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Tests/Find-SCEPmanCertificate.Tests.ps1 Outdated
Comment thread Tests/Revoke-SCEPmanCertificate.Tests.ps1 Outdated
Comment thread SCEPmanClient/Public/Find-SCEPmanCertificate.ps1
Comment thread SCEPmanClient/Public/Revoke-SCEPmanCertificate.ps1 Outdated
Comment thread SCEPmanClient/Public/Revoke-SCEPmanCertificate.ps1 Fixed

Copilot AI commented May 22, 2026

Copy link
Copy Markdown
Contributor

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • management.azure.com
    • Triggering command: /usr/bin/pwsh pwsh -NoLogo -NoProfile -Command Invoke-ScriptAnalyzer -Path . -Recurse (dns block)
    • Triggering command: /usr/bin/pwsh pwsh -NoLogo -NoProfile -Command ./Tests/Invoke-ProjectTests.ps1 (dns block)
    • Triggering command: /usr/bin/pwsh pwsh -NoLogo -NoProfile -Command Invoke-Pester -Path ./Tests/Find-SCEPmanCertificate.Tests.ps1 -main/dist/indexuser.name (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@cheinzler-gk cheinzler-gk changed the title Feat/revocation api Feat/Management API May 26, 2026
@cheinzler-gk cheinzler-gk requested a review from Copilot May 27, 2026 10:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Comment thread SCEPmanClient/Public/Revoke-SCEPmanCertificate.ps1 Outdated
Comment thread SCEPmanClient/Public/Revoke-SCEPmanCertificate.ps1 Outdated
Comment thread SCEPmanClient/Public/Revoke-SCEPmanCertificate.ps1 Outdated
Comment thread SCEPmanClient/Public/Find-SCEPmanCertificate.ps1 Outdated
Comment thread SCEPmanClient/Public/Find-SCEPmanCertificate.ps1

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread SCEPmanClient/Public/Revoke-SCEPmanCertificate.ps1
Comment thread SCEPmanClient/Public/Find-SCEPmanCertificate.ps1
@cheinzler-gk cheinzler-gk merged commit cc61b11 into main May 27, 2026
4 checks passed
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.

4 participants