Add Device Notes, Custom PSA, and SSO authentication support - #6
Conversation
- Get-NCDeviceNotes: list notes with pagination - New-NCDeviceNote: add note to single device or bulk (POST /devices/notes) - Set-NCDeviceNote: modify a note (PUT) - Remove-NCDeviceNote: delete single or batch notes
- New-NCCustomPsaTicket: create tickets (POST /custom-psa/tickets) - Get-NCCustomPsaTicket: now supports GET (no credentials) and POST variants - Invoke-NCCustomPsaTicket: reopen/resolve tickets
- Set-NCStandardPsaCustomerMapping: PUT customer mappings - Get-NCStandardPsaCompanies: list PSA companies - Get-NCStandardPsaContacts: list PSA contacts - Get-NCStandardPsaSites: list PSA sites
…d service actions - Get-NCCurrentUser: GET /api/users/me - Get-NCOrgLimits / Set-NCOrgLimits: get/patch org unit limits - New-NCRemoteControlTask / Get-NCRemoteControlType: remote control endpoints - New-NCUser: create user in org unit - Invoke-NCDeviceServiceAction: Windows service start/stop/restart
…nifest - Get-NCServerInfo -Time: GET /api/server-info/time - Get-NCApiLinks: add -AccessGroups, -ScheduledTasks, -Users switches - NCRestAPI.psd1: add 18 new functions to FunctionsToExport
- NCRestAPI class: add AuthenticateSso() method and UseSso flag - SSO constructor overload uses /api/auth/sso instead of /api/auth/authenticate - EnsureValidToken and RefreshAccessToken respect SSO mode for re-auth - Set-NCRestConfig: add -SsoToken parameter set - Connect-NCentral: add -SsoToken parameter set
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness and consistency issues in newly added cmdlets (parameter-set validity, password input validation, and inconsistent ID parameter types) that should be addressed before release.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR bumps the NCRestAPI PowerShell module to v1.9.0 and expands API coverage with new cmdlets for device notes, PSA integrations, org limits, remote control, user operations, and SSO-based authentication.
Changes:
- Added SSO authentication support via
Connect-NCentral -SsoToken/Set-NCRestConfig -SsoTokenand corresponding backend logic inNCRestAPI. - Introduced multiple new endpoint wrappers (device notes CRUD, org limits, standard/custom PSA actions, remote control tasks, service actions, current user lookup).
- Updated documentation, changelog, and module manifest to reflect the new cmdlets and version.
File summaries
| File | Description |
|---|---|
| README.md | Documents SSO auth and newly added cmdlets/endpoints. |
| Public/Set-NCStandardPsaCustomerMapping.ps1 | Adds standard PSA customer mapping update cmdlet. |
| Public/Set-NCRestConfig.ps1 | Adds parameter sets for API token vs SSO token and passes SSO mode to the client. |
| Public/Set-NCOrgLimits.ps1 | Adds org unit limits PATCH cmdlet. |
| Public/Set-NCDeviceNote.ps1 | Adds device note update cmdlet. |
| Public/Remove-NCDeviceNote.ps1 | Adds device note delete cmdlet (single + batch). |
| Public/New-NCUser.ps1 | Adds user creation cmdlet. |
| Public/New-NCRemoteControlTask.ps1 | Adds remote-control task creation cmdlet. |
| Public/New-NCDeviceNote.ps1 | Adds device note creation cmdlet (single + bulk). |
| Public/New-NCCustomPsaTicket.ps1 | Adds custom PSA ticket creation cmdlet. |
| Public/Invoke-NCDeviceServiceAction.ps1 | Adds Windows service action cmdlet for a device. |
| Public/Invoke-NCCustomPsaTicket.ps1 | Adds custom PSA ticket reopen/resolve cmdlet. |
| Public/Get-NCStandardPsaSites.ps1 | Adds standard PSA sites lookup cmdlet. |
| Public/Get-NCStandardPsaContacts.ps1 | Adds standard PSA contacts lookup cmdlet. |
| Public/Get-NCStandardPsaCompanies.ps1 | Adds standard PSA companies lookup cmdlet. |
| Public/Get-NCServerInfo.ps1 | Adds -Time switch for /api/server-info/time. |
| Public/Get-NCRemoteControlType.ps1 | Adds remote-control type lookup cmdlet. |
| Public/Get-NCOrgLimits.ps1 | Adds org unit limits GET cmdlet. |
| Public/Get-NCDeviceNotes.ps1 | Adds device notes GET cmdlet (paged + -All). |
| Public/Get-NCCustomPsaTicket.ps1 | Updates custom PSA ticket retrieval to support GET (no credential) and POST (credentialed). |
| Public/Get-NCCurrentUser.ps1 | Adds /api/users/me cmdlet. |
| Public/Get-NCApiLinks.ps1 | Expands hypermedia navigation switches to cover additional root link endpoints. |
| Public/Connect-NCentral.ps1 | Adds -SsoToken parameter set and forwards to Set-NCRestConfig. |
| Private/NCRestAPI.ps1 | Adds SSO authentication flow and refresh re-auth behavior for SSO mode. |
| NCRestAPI.psd1 | Bumps module version and exports new cmdlets. |
| CHANGELOG.md | Adds v1.9.0 entry describing new features/cmdlets. |
Review details
Suppressed comments (1)
Public/New-NCUser.ps1:120
- The password conversion falls back to
[string]$Passwordfor any non-securestring input, which will accept arbitrary objects and produce unintended passwords (e.g.,System.Object). Restrict this to[string]and throw for anything else.
$bstr = [Runtime.InteropServices.Marshal]::SecureStringToBSTR($Password)
try { $plainPassword = [Runtime.InteropServices.Marshal]::PtrToStringBSTR($bstr) }
finally { [Runtime.InteropServices.Marshal]::ZeroFreeBSTR($bstr) }
- Files reviewed: 26/26 changed files
- Comments generated: 7
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…inks Address PR review: DeviceId/OrgUnitId/CustomerId/PsaCompanyId changed from [int] to [string] for consistency with existing module convention. Added explicit Root parameter set to Get-NCApiLinks.
There was a problem hiding this comment.
🔵 Needs a closer look
SSO authentication is currently broken (authorization header placeholder) and several new device-facing cmdlets use inconsistent DeviceId types compared to established module patterns.
Review details
Suppressed comments (5)
Previously missed (1) — in code that hasn't changed since the last review.
Private/NCRestAPI.ps1:118
- SSO authentication introduces a new code path (
AuthenticateSso()plus theUseSsobranching in refresh/ensure logic), but there are no Pester tests covering it (the existing test suite doesn’t mentionSsoToken/AuthenticateSso). Adding tests that mockInvoke-RestMethodto validate the/api/auth/ssocall, headers, and fallback behavior would prevent regressions.
This issue also appears on line 109 of the same file.
[void] AuthenticateSso() {
$this.Log("[NCRESTAPI] AuthenticateSso: starting.")
$url = "$($this.BaseUrl)/api/auth/sso"
$headers = @{
'Accept' = '*/*'
'Authorization' = "Bearer $($this.Reveal($this.ApiToken))"
}
if ($this.RefreshTokenExpiration) { $headers['X-REFRESH-EXPIRY-OVERRIDE'] = $this.RefreshTokenExpiration }
if ($this.AccessTokenExpiration) { $headers['X-ACCESS-EXPIRY-OVERRIDE'] = $this.AccessTokenExpiration }
try {
$response = Invoke-RestMethod -Uri $url -Headers $headers -Method Post -TimeoutSec $this.TimeoutSec
Private/NCRestAPI.ps1:113
AuthenticateSso()is not using the provided SSO token: theAuthorizationheader is currently a literal placeholder ("******"), so the POST to/api/auth/ssowill always be unauthenticated.
$url = "$($this.BaseUrl)/api/auth/sso"
$headers = @{
'Accept' = '*/*'
'Authorization' = "Bearer $($this.Reveal($this.ApiToken))"
}
Public/New-NCRemoteControlTask.ps1:29
- This module typically models
DeviceIdparameters as[string](e.g.Get-NCDevicesuses[string]$DeviceId), but this new cmdlet uses[int]. Usingstringavoids potential Int32 overflow and keeps pipeline/property-name binding consistent across cmdlets.
[Parameter(Mandatory, ValueFromPipelineByPropertyName)]
[ValidateNotNullOrEmpty()]
[string]$DeviceId,
Public/Invoke-NCDeviceServiceAction.ps1:26
- This module typically models
DeviceIdparameters as[string](e.g.Get-NCDevicesuses[string]$DeviceId), but this new cmdlet uses[int]. Switching tostringkeeps device ID types consistent across cmdlets and avoids potential Int32 overflow.
[Parameter(Mandatory, ValueFromPipelineByPropertyName)]
[ValidateNotNullOrEmpty()]
[string]$DeviceId,
Public/Get-NCRemoteControlType.ps1:21
- This module typically models
DeviceIdparameters as[string](e.g.Get-NCDevicesuses[string]$DeviceId), but this cmdlet uses[int]. Usingstringavoids potential Int32 overflow and keeps parameter types consistent across device-related cmdlets.
[Parameter(Mandatory, ValueFromPipelineByPropertyName)]
[ValidateNotNullOrEmpty()]
[string]$DeviceId
)
- Files reviewed: 26/26 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
A few newly added cmdlets need tighter client-side validation and password-handling adjustments to avoid preventable API errors and reduce plaintext credential exposure.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (5)
Previously missed (5) — in code that hasn't changed since the last review.
Public/Set-NCRestConfig.ps1:56
- Set-NCRestConfig accepts ApiToken/SsoToken as mandatory but doesn’t validate they’re non-empty. This allows calling Set-NCRestConfig directly with an empty string token, producing harder-to-diagnose auth failures; Connect-NCentral already validates these tokens as not null/empty.
[Parameter(Mandatory, ParameterSetName = 'ApiToken')]
[object]$ApiToken,
[Parameter(Mandatory, ParameterSetName = 'SsoToken')]
[object]$SsoToken,
Public/Invoke-NCDeviceServiceAction.ps1:32
- The API spec limits service actions to Start/Stop/Restart/Pause/Resume. Without validating Action, callers can pass invalid values and only discover the mistake via a 400 from the server.
[Parameter(Mandatory)]
[ValidateNotNullOrEmpty()]
[string]$Action
Public/New-NCRemoteControlTask.ps1:30
- The API spec defines a fixed enum for remoteControlType (SSH/VNC/RDP/MSPAnywhere/Telnet). Adding a ValidateSet here will fail fast client-side and prevent avoidable 400 responses.
[string]$RemoteControlType,
Public/New-NCUser.ps1:119
- SecureString password is converted to plaintext before ShouldProcess is evaluated, so -WhatIf / user-decline still materializes the password in memory. Consider moving the ShouldProcess check above the SecureStringToBSTR conversion to minimize plaintext exposure.
$bstr = [Runtime.InteropServices.Marshal]::SecureStringToBSTR($Password)
try { $plainPassword = [Runtime.InteropServices.Marshal]::PtrToStringBSTR($bstr) }
finally { [Runtime.InteropServices.Marshal]::ZeroFreeBSTR($bstr) }
Public/New-NCUser.ps1:145
- The plaintext password is kept in $plainPassword after the request completes. Clearing it in a finally block reduces the chance of it being captured in later debugging/inspection.
if (-not $PSCmdlet.ShouldProcess($Email, 'Create user')) { return }
$api.Post("api/org-units/$OrgUnitId/users", $body)
- Files reviewed: 26/26 changed files
- Comments generated: 1
- Review effort level: Lite
| [int]$PsaCompanyId, | ||
| [int]$PsaSiteId, | ||
| [int]$PsaContactId |
This pull request introduces a new minor release (v1.9.0) with significant feature additions and improvements across the module. Key highlights include support for device notes (full CRUD), SSO authentication, expanded PSA integration, new endpoints for remote control and org unit limits, and enhanced API discoverability.
Major feature additions:
Get-NCDeviceNotes,New-NCDeviceNote,Set-NCDeviceNote,Remove-NCDeviceNote) including bulk operations. [1] [2]Connect-NCentral -SsoTokenandSet-NCRestConfig -SsoToken, with backend changes to support SSO token authentication and refresh logic. [1] [2] [3] [4] [5] [6] [7]New-NCCustomPsaTicket(create),Invoke-NCCustomPsaTicket(reopen/resolve), and updatedGet-NCCustomPsaTicketto support both credential-free GET and credentialed POST variants. [1] [2] [3]Set-NCStandardPsaCustomerMapping,Get-NCStandardPsaCompanies,Get-NCStandardPsaContacts,Get-NCStandardPsaSites,Get-NCOrgLimits,Set-NCOrgLimits). [1] [2] [3]New-NCRemoteControlTask,Get-NCRemoteControlType,New-NCUser, andGet-NCCurrentUserfor enhanced remote and user operations. [1] [2] [3]API and discoverability improvements:
Get-NCApiLinks: Now covers all navigation endpoints with new switches (-AccessGroups,-ScheduledTasks,-Users). [1] [2] [3]Get-NCServerInfo: Added support for retrieving server time with the-Timeswitch. [1] [2] [3] [4]Versioning and manifest:
See the updated
CHANGELOG.mdfor a full list of new commands and features.