[pull] master from KelvinTegelaar:master#23
Open
pull[bot] wants to merge 83 commits into
Open
Conversation
When AppCache.ApplicationId differs from the Key Vault-backed ApplicationID, Get-GraphToken now reloads auth and updates the AppCache marker to the KV value. This prevents repeated auth reloads on every token request when AppCache and KV are out of sync.
Expand the NudgeMFA standard to support: - Multiple authentication methods (Authenticator, Passkey) - Group-level include/exclude targeting with group name resolution - Snooze enforcement limits (max 3 snoozes) Adds Set-CIPPRegistrationCampaign helper function as a single writer for the campaign, shared by the new ExecRegistrationCampaign HTTP endpoint and the enhanced NudgeMFA standard. The standard now supports both portal-configured and template-specified group targeting, with full compliance comparison.
Expand the NudgeMFA standard to support: - Multiple authentication methods (Authenticator, Passkey) - Group-level include/exclude targeting with group name resolution - Snooze enforcement limits (max 3 snoozes) Adds Set-CIPPRegistrationCampaign helper function as a single writer for the campaign, shared by the new ExecRegistrationCampaign HTTP endpoint and the enhanced NudgeMFA standard. The standard now supports both portal-configured and template-specified group targeting, with full compliance comparison. Backwards compatible with any current configuration
Update the default assignment mode to 'append' for a less destructive default. Frontend PR: KelvinTegelaar/CIPP#6342
Resolves KelvinTegelaar/CIPP#6349, "[Feature Request]: Auto-create NinjaOne vulnerability scan group during CVE sync". Previously, `Invoke-NinjaOneTenantSync` looked up the NinjaOne CVE vulnerability scan group by name and failed the CVE sync step for the tenant if that scan group did not already exist in NinjaOne. This forced admins to manually pre-create the scan group in every tenant before CVE sync could function. - Extract the scan-group lookup into a new, independently testable helper `Resolve-NinjaOneCveScanGroup` (Private/NinjaOne). It now: - Looks up the scan group by configured name. - Auto-creates it via the NinjaOne API when not found, using the configured (or default) device-id/cve-id header names. - Returns $null and logs an Error via Write-LogMessage when lookup and creation both fail, so the outer CVE sync block can log and continue instead of throwing. - Wire `Invoke-NinjaOneTenantSync`'s CVE sync block to call the new helper instead of inline lookup-or-fail logic. - Fix an unrelated but adjacent bug: the NinjaOne API Authorization header was being built without the required "Bearer " prefix (`"Bearer $($Token.access_token)"`), which would have caused every authenticated NinjaOne API call to fail with 401 Unauthorized. - Address PR review feedback (review comment r3581392606) on `Resolve-NinjaOneCveScanGroup`: - Wrap the initial scan-group lookup GET request in its own try/catch. Previously an uncaught exception here (e.g. 401, timeout) would propagate out of the function, contradicting its documented `$null`-on-failure contract; it now logs an Error-severity message and returns $null like the create-failure path already did. - Pipe the `Where-Object` name match through `Select-Object -First 1` so the function always returns a single scan-group object as documented, even if NinjaOne has multiple scan groups sharing the same name (previously it could return an array in that case, silently breaking downstream `.id`/`.deviceIdHeader`/`.cveIdHeader` access and upload-URI construction in the caller). Tests: - Add `Tests/NinjaOne/Resolve-NinjaOneCveScanGroup.Tests.ps1`: 5 scenarios covering existing scan group found, auto-create on missing, auto-create using custom header names, auto-create using default header names, and the not-found/creation-failure path. Plus 2 new scenarios added for the review-feedback fix: the initial lookup GET failing (returns $null, logs Error, does not throw, does not attempt create), and multiple scan groups sharing the same name (returns a single object, not an array). - Add `Tests/NinjaOne/Invoke-NinjaOneTenantSync.Tests.ps1`: a comprehensive Pester suite for the full sync function (16 scenarios) covering successful sync, tenant match validation, hostname allow-list validation, CVE sync (including the new auto-create path, exception filtering, and isolated failure handling), UserDocuments/LicenseDocuments toggles, final custom-fields PATCH failure handling, and the tenant-sync concurrency guard. While writing the concurrency-guard test, discovered a genuine, pre-existing production bug: the guard parses the stored `lastStartTime` (a UTC ISO-8601 string) with `Get-Date($string)`, which returns a `Kind=Local` DateTime, then compares it directly against `(Get-Date).ToUniversalTime()` (`Kind=Utc`). .NET's DateTime comparison operators ignore `Kind` and compare raw ticks, so the "already running" check is silently wrong by the host's UTC offset on any non-UTC host. This is out of scope for #6349 and is tracked separately as KelvinTegelaar/CIPP#6351; the test documents the current (buggy) comparison behavior deterministically across host timezones rather than masking or silently working around it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Convert `AgeLimitForRetention` from `TimeSpan.TotalDays` to an integer before state comparison in `Invoke-CIPPStandardRetentionPolicyTag`. This avoids decimal day values causing false drift detection when validating retention policy tag settings.
Update Smart Lockout metadata and docs to use the correct on-premises mode value (`Enforce` instead of `Enforced`). In `Invoke-CIPPStandardSmartLockout`, add compatibility mapping for legacy templates that still store `Enforced`, converting it to `Enforce` before compliance checks and remediation calls so tenants are not incorrectly flagged as non-compliant.
Use the actual policy properties returned by the test when building the failed policy table, and hardcode the expected action as `Quarantine` so the report matches the check logic.
Update ORCA121 to inspect hosted content filter policies instead of quarantine policies and flag SpamAction or PhishSpamAction values that Zero Hour Autopurge cannot remediate. This adds clearer pass/fail output with per-setting details and documents the test intent.
Add field-level projection to Get-CIPPTestData and New-CIPPDbRequest to reduce memory usage by only materializing queried fields. Introduce Get-CippTestDataFieldManifest to manage per-type field whitelists, and replace ConvertFrom-Json with CIPP.CippJson (System.Text.Json-based) for faster parsing. Update Get-CippDbRoleMembers with missing fields (appId, EndDateTime, IsPermanent) required by role member callers. Refactor CIPPTestDataCache to use direct PSObject APIs instead of reflection for heap-size estimation.
Add the required RoleManagement permissions to SAMManifest and update PIM cache jobs to use app-only Graph calls (`-AsApp`) so role APIs stop failing with unauthorized delegated access. Role assignment and eligibility schedule caching now expands `principal` to preserve member identity fields, and role management policy caching now reads `roleManagementPolicyAssignments` (with required filter/expands) and flattens policy rules/effectiveRules alongside `roleDefinitionId` for role-to-policy lookups.
Fix multiple bugs in CIPP test functions where incorrect field names or IDs were used when querying role data and policies: - Replace `$GA.id` with `$GA.roleTemplateId` when matching PIM role assignments (CIS_1_1_2, CIS_1_1_3) - Replace `$Role.templateId` with `$Role.roleTemplateId` when filtering roles and policies (ZTNA21813, 21816, 21818, 21820) - Fix app protection policy tests to read from correct data type and distinguish between missing data vs. missing platform-specific policy (ZTNA24548, ZTNA24549) - Replace `IsPermanent` for permanent GA detection instead of `AssignmentType -eq 'Permanent'` (ZTNA21835) - Use correct field names from Get-CippDbRoleMembers: `id` instead of `principalId`, `displayName` instead of `principalDisplayName` (ZTNA21835, ZTNA21836) - Fix role management policy lookup to use `roleDefinitionId` instead of nonexistent `effectiveRules.target.targetObjects.id` (ZTNA21819, ZTNA21820) - Clean up Entra URL construction (ZTNA21836) These changes fix tests that were silently returning false negatives due to incorrect field references.
…ching (#6347) The drift report failed to detect Intune configuration deviations because Get-CIPPDrift.ps1 compared template and tenant policy names using `.displayName -eq .displayName` alone. When both sides had no `.displayName` property (common for Graph policy types that only expose `.name`), this evaluated `$null -eq $null` as true, causing every such tenant policy to be incorrectly matched against the first Intune template in the loop and suppressing real drift. The fix restores all four original name/displayName pairings (displayName<->displayName, name<->name, displayName<->name, name<->displayName), with each pairing individually null-guarded so a match only counts when both sides have a real, non-null value. This is a strict superset of the prior matching capability - it only excludes the null-eq-null case that caused the false positive - so Settings Catalog and other name-only policy types deployed from a template continue to match correctly on name<->name. Adds Tests/Reports/Get-CIPPDrift.Tests.ps1, a new Pester suite (16 tests) covering: - the original null-eq-null false-match bug - each of the four name/displayName pairing combinations - the Settings Catalog name<->name regression path - multi-template alignment loops (matching a later template when earlier ones don't match) - Conditional Access template matching - standards deviation display-name/description resolution - stale drift-entity pruning Fixes KelvinTegelaar/CIPP#6347
Add CopilotPolicySettings.ReadWrite to the permissions translator for both Delegated and Application origins, and include the corresponding scope/role IDs in SAMManifest requiredResourceAccess. This updates the SAM app permission set so Copilot policy settings can be managed in both signed-in and app-only flows.
Remove the scheduled SharePointSharingLinks cache task from Push-CIPPDBCacheData and leave it as an ad-hoc-only operation, since full sharing-link enumeration can run too long on large tenants.
…ching (#6347) (#2143) ## What Fixes drift report failing to detect Intune configuration deviations (#6347). `Get-CIPPDrift.ps1` matched an Intune template to a tenant policy using `.displayName -eq .displayName` only. Graph policy types that expose only a `.name` property (no `.displayName`) evaluated `$null -eq $null` as `true`, so every such tenant policy silently matched the first template in the loop regardless of actual content - suppressing real drift detection entirely for those policy types. ## Fix Restores all four original name/displayName pairings used before the regression was introduced, each individually null-guarded so a match only counts when both sides have a real, non-null value: - `displayName <-> displayName` - `name <-> name` - `displayName <-> name` - `name <-> displayName` This is a strict superset of the prior matching capability - the only behavior removed is the null-eq-null false match. Settings Catalog and other name-only policy types deployed from a template still match correctly via `name <-> name`. ## Tests Adds `Tests/Reports/Get-CIPPDrift.Tests.ps1`, a new Pester suite (16 tests) covering: - the original null-eq-null false-match bug - each of the four name/displayName pairing combinations - the Settings Catalog name<->name matching path - multi-template alignment loops (matching a later template when earlier ones don't match) - Conditional Access template matching - standards deviation display-name/description resolution - stale drift-entity pruning All 16 tests pass locally via `Invoke-Pester -Path Tests\Reports\Get-CIPPDrift.Tests.ps1`. Fixes KelvinTegelaar/CIPP#6347
…#2141) Resolves KelvinTegelaar/CIPP#6349, "[Feature Request]: Auto-create NinjaOne vulnerability scan group during CVE sync". ## Summary Previously, `Invoke-NinjaOneTenantSync` looked up the NinjaOne CVE vulnerability scan group by name and failed the CVE sync step for the tenant if that scan group did not already exist in NinjaOne. This forced admins to manually pre-create the scan group in every tenant before CVE sync could function. - Extract the scan-group lookup into a new, independently testable helper `Resolve-NinjaOneCveScanGroup` (`Private/NinjaOne`). It now: - Looks up the scan group by configured name. - Auto-creates it via the NinjaOne API when not found, using the configured (or default) device-id/cve-id header names. - Returns `$null` and logs an Error via `Write-LogMessage` when lookup and creation both fail, so the outer CVE sync block can log and continue instead of throwing. - Wire `Invoke-NinjaOneTenantSync`'s CVE sync block to call the new helper instead of inline lookup-or-fail logic. - Fix an unrelated but adjacent bug: the NinjaOne API Authorization header was being built without the required `Bearer ` prefix, which would have caused every authenticated NinjaOne API call to fail with 401 Unauthorized. ## Tests - `Tests/NinjaOne/Resolve-NinjaOneCveScanGroup.Tests.ps1`: 5 scenarios covering existing scan group found, auto-create on missing, auto-create using custom header names, auto-create using default header names, and the not-found/creation-failure path. - `Tests/NinjaOne/Invoke-NinjaOneTenantSync.Tests.ps1`: a comprehensive Pester suite for the full sync function (16 scenarios) covering successful sync, tenant match validation, hostname allow-list validation, CVE sync (including the new auto-create path, exception filtering, and isolated failure handling), UserDocuments/LicenseDocuments toggles, final custom-fields PATCH failure handling, and the tenant-sync concurrency guard. While writing the concurrency-guard test, discovered a genuine, pre-existing production bug: the guard parses the stored `lastStartTime` (a UTC ISO-8601 string) with `Get-Date($string)`, which returns a `Kind=Local` DateTime, then compares it directly against `(Get-Date).ToUniversalTime()` (`Kind=Utc`). .NET's DateTime comparison operators ignore `Kind` and compare raw ticks, so the "already running" check is silently wrong by the host's UTC offset on any non-UTC host. **This is out of scope for #6349** and is tracked separately as #4; the test documents the current (buggy) comparison behavior deterministically across host timezones rather than masking or silently working around it. Both suites pass 100% (5/5 and 16/16). PSScriptAnalyzer run clean on all touched/new files (only pre-existing repo-wide style warnings, no errors).
Refactor Set-CIPPUser to use a shared helper that decides whether group membership changes should use Exchange cmdlets based on `addedFields.calculatedGroupType`, with fallback to legacy `groupType` values. This fixes inconsistent add/remove behavior caused by the previous inline condition and variable-case mismatch, ensuring distribution lists and mail-enabled security groups are handled consistently.
Signed-off-by: KelvinTegelaar <49186168+KelvinTegelaar@users.noreply.github.com>
Detect AADSTS7000229 token-acquisition failures in audit search creation and classify them as `AccessDenied` instead of generic transient errors. Update the webhook flow to handle `AccessDenied` like `AuditingDisabled`: cache the tenant in `AuditLogDisabledTenants` for 24 hours, stop further processing in the cycle, and defer remaining windows with a consistent bail reason.
…llowlists The allowlist of special includeUsers/excludeUsers values spelled the Graph value as 'GuestOrExternalUsers' (singular), but Microsoft's real value is 'GuestsOrExternalUsers' (plural). In New-CIPPCAPolicy the misspelled value fell through to the display-name lookup, found no matching user, and was silently dropped - producing an empty includeUsers array that Graph rejects on deploy. Also corrects the same spelling in the New-CIPPCATemplate ID-to-name maps (benign fallthrough there, fixed for consistency), and logs a warning naming any user value that fails to resolve instead of dropping it silently.
… them Eleven standards wrapped their remediation calls in try/catch with an empty catch block, so any failure from Set-CIPPAuthenticationPolicy (which throws on error) was silently discarded and the standard appeared to succeed.
…/assignments App Protection / managed-app policies reported NON-COMPLIANT on every drift run regardless of settings: templates are stored with apps[], deployment strips apps (Set-CIPPIntunePolicy), and the policy read-back never returns apps, assignments, or a matching deployedAppCount (Get-CIPPIntunePolicy has no $expand), so the generic deep-compare always flagged them. - Exclude 'assignments' globally: read-back never includes assignments for any policy type, and assignment verification has its own dedicated path (Compare-CIPPIntuneAssignments via verifyAssignments). - Exclude 'apps' and 'deployedAppCount' only for CompareType 'AppProtection' (what the Standards/drift engine passes), because Device configs carry legitimate nested 'apps' arrays (e.g. kiosk profiles) that must keep being compared. - '@odata.context' already skipped by the existing *@OData* wildcard.
Templates synced from a gold tenant fetch App Protection policies via
their concrete type URL (e.g. androidManagedAppProtections('id')), and
Graph omits @odata.type from those responses. The stored RAWJson then
lacks @odata.type, so Set-CIPPIntunePolicy built the deploy URL as
deviceAppManagement/s and Graph returned "Resource not found for the
segment 's'.". Re-add @odata.type from the known ODataType when Graph
omits it, and fail with a clear error on deploy when a stored template
still has no @odata.type.
Two defects allowed Invoke-SherwebMigration scheduled tasks to keep firing daily (sending alerts and performing real Sherweb purchases and Pax8 subscription cancellations) after the Sherweb migration was disabled: - Invoke-SherwebMigration had no enabled-guard: it read the Sherweb extension config but never checked Enabled before acting. It now returns immediately unless the Sherweb config exists and Enabled is true, which neutralizes any stale scheduled task. - Register-CIPPExtensionScheduledTasks created migration tasks in the enable branch, but the disable cleanup only filtered Push-CippExtensionData tasks, so migration tasks were never removed. Migration tasks are now queried at function scope, removed when the Sherweb extension is disabled, and removed when a tenant is no longer Sherweb-mapped.
…ve licensed users alert The alert already fetched assignedLicenses and accountEnabled from Graph but dropped them from the emitted alert data. Surface license display names, raw SKU IDs, account enablement and days since last sign-in so the alert can be actioned without a follow-up lookup.
…isfy its own check The remediation enabled the MicrosoftAuthenticator method but never passed the feature settings the compliance check grades against, so the standard could never converge: displayAppInformationRequiredState was checked but not set, and numberMatchingRequiredState was checked even though Set-CIPPAuthenticationPolicy deliberately strips it from the PATCH body (Microsoft now enforces number matching and the setting can no longer be toggled). The empty catch around the remediation call also swallowed every error, leaving no signal in the logs. - Pass -MicrosoftAuthenticatorDisplayAppInfo 'enabled' in the remediation so the PATCH sets what the check grades - Drop numberMatchingRequiredState from the check and the report comparison, matching Microsoft's enforced-by-default behavior - Replace the empty catch with Get-CippException + Write-LogMessage error logging, matching the idiom in Invoke-CIPPStandardAuthenticationMethods
Update New-CIPPCATemplate to use the correct special value `GuestsOrExternalUsers` when processing include/exclude users and groups. This fixes a typo that could cause the selector to be treated like a normal ID instead of being preserved.
Update request parsing to support both PSCustomObject and IDictionary shapes from Azure Functions deserialization. `Get-CippMcpToolResult` now correctly flattens MCP arguments and clones headers when they are dictionaries, preventing lost params/headers (including EasyAuth context). `Invoke-ListMailboxes` now reads query parameter names from dictionary keys, so allowed mailbox filters are applied reliably.
… sending null
Untoggled "switch" inputs are omitted from a standard's settings, so
TeamsGlobalMeetingPolicy, TeamsExternalFileSharing, and
TeamsFederationConfiguration serialized null booleans to the Teams
ConfigApi, which rejects the write with 400 "Error converting value
{null} to type 'System.Boolean'". The null also poisoned the
$StateIsCorrect comparison ($CurrentState.X -eq $null is never true),
so affected tenants reported non-compliant and failed remediation on
every run.
Coalesce each switch to $false (the CIS recommended value, matching the
convention in TeamsExternalAccessPolicy/TeamsGuestAccess/
TeamsEmailIntegration) and use the coalesced variable in the state
comparison, remediation payload, and report expected values. In
TeamsExternalFileSharing the existing "?? $false" was dead code because
-eq binds tighter than ??.
Replace the null-check wrapper with a direct null filter when building `ComparisonResults`. This ensures the endpoint always returns an array containing only valid comparison entries, avoiding accidental null items in the response.
Add `Sync-CippContainerUpdateState` and use it in both timer and container management endpoints so stale "update available" results are corrected after a restart. The update-check flow now also captures and preserves `RemoteBuildDate` and `CheckedTag`, and the settings/status response includes build-date metadata for both running and remote images.
Ensure container image build timestamps are converted from PowerShell DateTime values into UTC ISO 8601 strings before they are returned or stored. This avoids ambiguous unspecified kinds and keeps timer-driven update checks and container management responses consistent.
Resolve container update settings through the shared sync path so never-saved fields default to auto-restart on, hourly checks, and 23:00 while still respecting explicit disabled or empty values. Also tighten status reconciliation after restarts and fix check-time validation/formatting so early UTC hours are accepted correctly.
Refactors the SSO migration checks in `Test-CIPPAccess` to use explicit permission flags. This keeps the forced migration prompt limited to users who can manage app settings and skips migration lookups for users with no assigned permissions.
Add a shared Autopilot profile name validator and use it in both the HTTP endpoint and default deployment profile flow. This catches unsupported Intune characters before submission so the API returns a clear bad request or logged error instead of an opaque service-side 500.
Add `Resolve-CIPPDlpAdvancedRule` and apply it across template capture, policy deploy, and drift comparison so DLP rules keep either `AdvancedRule` or flat condition fields, never both. Update DLP rule field metadata to expose `RuleConditions` and include `AdvancedRule`, then normalize advanced-rule JSON during comparison to avoid false drift from formatting or key-order differences.
… missing" This reverts commit 24db2c5.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )