PowerBi support.. Add automated tests for PowerBi to ScubaGear#2067
Open
skirkpatrickMSFT wants to merge 48 commits intomainfrom
Open
PowerBi support.. Add automated tests for PowerBi to ScubaGear#2067skirkpatrickMSFT wants to merge 48 commits intomainfrom
skirkpatrickMSFT wants to merge 48 commits intomainfrom
Conversation
- Add SPORestHelper.psm1 with direct REST API calls to SharePoint Admin API - Remove dependency on Microsoft.Online.SharePoint.PowerShell and PnP.PowerShell - Support both interactive (browser auth) and certificate-based authentication - Update Rego policies to reference 'SharePoint REST API' - Update documentation and test fixtures
- Replace Write-Host with Write-Information - Remove unused parameters (PnPFlag, TenantDomain) - Remove unused CodeChallenge variable - Fix null comparison order (PSPossibleIncorrectComparisonWithNull) - Add OutputType attributes - Remove trailing whitespace - Remove obsolete comments from RequiredVersions.ps1
Document the browser-based OAuth flow for SharePoint that requires users to paste the redirect URL
PnPFlag was removed from ExportSharePointProvider but Orchestrator was still passing it
… property retrieval - Fix switch statement syntax in SPOSiteHelper.psm1 that incorrectly matched all environments to commercial/gcc URLs - Change Write-Warning to Write-Verbose in SPORestHelper.psm1 for non-critical site property retrieval failures
… connection for sharepoint preconditions.
…oreAll, add Set-SPOTenant REST wrapper, remove stale PNP test name.
…P and SPO helpers.
…op-level environments field.
…used by Rego (ANYONE=2, EXISTINGGUESTS=3).
…ess=3 to match REST API/Rego values.
…into two sequential REST calls.
…with any other fields.
…Scuba tests; add ODBSharingCapability to Set-SPOTenant REST wrapper.
…ld) and 2016-11-01 to avoid GCC High schema limitation; update test preconditions to match by displayName.
…ScriptAnalyzer warning.
…ability to avoid SPO 500 (ODB cannot exceed tenant sharing level).
…alid in 2016-11-01 schema.
…esn't recognize modern field names; 2019-10-01 works now that environments field is removed).
…ts displayName/environmentType/connectorGroups; only 'environments' was unsupported, which we no longer send).
…on body format and normalize provider response New-AdminDlpPolicy was sending a flat body (displayName, environmentType, connectorGroups) to the 2016-11-01 endpoint, which treats the body as an ApiPolicyDefinition and does not recognize 'displayName' at root level. Fix: use the ARM resource envelope (properties.definition with apiGroups/rules), omitting environmentFilter1 to create an AllEnvironments policy. Get-PowerPlatformDlpPoliciesRest was returning the raw 2016-11-01 ARM response (properties.displayName, properties.definition) but Rego accesses flat fields (displayName, environmentType, environments) directly on each policy value. Fix: normalize the response to flat format before returning.
…or test plan filters The apiPolicies REST API returns ARM format (properties.displayName), but test plan preconditions filter by \.displayName (flat, matching original cmdlet output). The filter matched nothing, so Remove-DlpPolicy was never called, the policy was never deleted, and the non-compliant case always returned true instead of false. Also change GET back to api-version=2016-11-01 to match the provider.
The ARM id for an environment-scoped DLP policy is a full path like /providers/.../environments/Default-xxx/apiPolicies/guid. Using only the bare GUID constructs a different (tenant-scoped) URL that silently no-ops. - Get-DlpPolicy now exposes the id field alongside name/displayName - Remove-DlpPolicy detects a full ARM path vs bare GUID and builds the correct DELETE URL; falls back to tenant-scoped path for bare GUIDs - Test plan precondition now pipes id (not name) as PolicyName to Remove-DlpPolicy - DELETE api-version changed from 2019-10-01 to 2016-11-01 to match GET Verified locally: policy is deleted and absent on re-query.
…nt network errors When RunScuba calls Get-PowerPlatformAccessToken (or interactive variant), MSAL makes an HTTP request to the auth endpoint. Occasional transient network errors on GitHub-hosted runners cause this to fail with 'An error occurred while sending the request', leaving AccessToken empty and all provider calls failing. Retry up to 3 times with a 5-second delay before giving up.
…errors Same pattern as Power Platform fix: MSAL HTTP requests to login.microsoftonline.us occasionally fail with 'An error occurred while sending the request' on GitHub-hosted runners, leaving AccessToken empty and causing Invoke-SCuBA to crash with a fatal report creation error. Retry up to 3 times with 5s delay.
The AllEnvironments DLP policy created in the compliant test was never removed, leaving it active for all subsequent policy tests. On some tenants this causes downstream precondition REST calls (Set-TenantSettings, Set-PowerAppTenantIsolationPolicy) to fail with 'Unable to connect to the remote server' because the active DLP policy restricts connector/API traffic. Add a postcondition to delete the policy after the compliant test.
Transient 'Unable to connect to the remote server' errors on the tenant isolation policy endpoint cause 3.1v1 preconditions to fail on some tenants. Add 3-attempt / 5-second-delay retry loop matching the pattern already used for token acquisition.
…t-PowerBITenantDetail, commented-out line
skirkpatrickMSFT
changed the title
Collaborator
|
We need to move the authentication logic for Get-PowerBIAccessToken and Get-PowerBIAccessTokenInteractive out of the provider and into Connection.psm1. See this comment on the sister PR for details. |
Collaborator
|
I noticed that Export-PowerBIProvider uses TryCommand to call the REST API whereas in the sister PR, TryCommand is not used. We should be consistent unless there is a reason to make one provider work differently from the other. My understanding of TryCommand is that it was designed to make it easy to track the success or failure of individual cmdlets (which in this case would be individual REST API calls) using consistent tracking objects and consistent error handling / error message logic. Therefore if there is a reason to avoid using TryCommand, let's discuss it. Here is what the pattern looks like in this PR (very similar to existing providers that use TryCommand): 
Here is what the pattern looks like in the sister PR for Sharepoint / PP (notice that TryCommand is not used): |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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.
🗣 Description
This adds PowerBI checks to ScubaGear using REST calls
💭 Motivation and context
The addition of PowerBI to ScubaGear checks
🧪 Testing
Extensive testing in GCCH and commercial
✅ Pre-approval checklist
✅ Pre-merge checklist
PR passed smoke test check.
Feature branch has been rebased against changes from parent branch, as needed.
Use
Update branchbutton below or use this reference to rebase from the command line.Resolved all merge conflicts on branch.
Squash all commits into one PR level commit using the
Squash and mergebutton.✅ Post-merge checklist