feat(ui): onboard Azure subscriptions from a Management Group - #12386
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAzure Management Group onboarding now supports tenant discovery, management-group hierarchies, subscription selection, apply payloads, credential handling, error messages, deletion flows, and integration coverage. ChangesAzure organization onboarding
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
✅ All required changelog fragments are present. |
Contributor
|
✅ No Conflicts No conflict markers, and the branch merges cleanly into its base. |
Contributor
🔒 Container Security ScanImage: ✅ No Vulnerabilities DetectedThe container image passed all security checks. No known CVEs were found.📋 Resources:
|
Contributor
🔎 Container Security Scan (Grype)Image: ✅ Nothing BlockingNo findings at critical or high severity. 📋 Resources:
|
6 tasks
pfe-nazaries
force-pushed
the
feature/azure-org-onboarding-ui
branch
from
August 7, 2026 11:36
d5641aa to
7ec278a
Compare
pfe-nazaries
marked this pull request as ready for review
August 10, 2026 07:26
Alan-TheGentleman
left a comment
Alan-TheGentleman
left a comment
Contributor
There was a problem hiding this comment.
The Azure organization name contract is inconsistent. The form presents the field as optional and says an empty value falls back to the name stored in Azure, but creation actually falls back to the tenant ID, while editing passes an empty string to updateOrganizationName(), which rejects it.
Please align this contract in both creation and editing. Either implement the Azure display-name fallback that the UI promises, or change the copy and validation so the organization name is required and handled consistently.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ui/lib/organizations.test.ts (1)
120-136: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
shortenNodeId.
ui/lib/organizations.tsexports the newshortenNodeIdfunction. This suite covers every other exported helper of that module, but notshortenNodeId. Two branches carry behavior that a future edit can break silently: the case-insensitive match on the ARM prefix, and theundefinedresult for AWS OU ids and GCP folder refs.org-account-tree-item.tsxdepends on theundefinedresult to keep showing the canonical id.♻️ Proposed test block
+describe("shortenNodeId", () => { + it("returns the trailing name of a management-group resource id", () => { + expect( + shortenNodeId( + "/providers/Microsoft.Management/managementGroups/engineering", + ), + ).toBe("engineering"); + }); + + it("matches the ARM prefix case-insensitively", () => { + expect( + shortenNodeId( + "/providers/microsoft.management/managementgroups/engineering", + ), + ).toBe("engineering"); + }); + + it("returns undefined for ids that are already short", () => { + expect(shortenNodeId("ou-abcd-12345678")).toBeUndefined(); + expect(shortenNodeId("folders/123456789012")).toBeUndefined(); + }); +});Add
shortenNodeIdto the import list at line 10.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/lib/organizations.test.ts` around lines 120 - 136, Add shortenNodeId to the organizations test import and add coverage for its ARM prefix matching, including case-insensitive input, plus assertions that AWS OU IDs and GCP folder references return undefined. Keep the existing canonical-ID behavior validated for unsupported formats.ui/components/providers/organizations/org-account-tree-item.tsx (1)
82-101: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the canonical id in the accessible name instead of
aria-label.
aria-labelis not allowed on the implicitgenericrole used by the plain<span>, so screen-reader support cannot rely on it. Add the canonicalvaluein an element with a valid accessible name; ifshortenedis shown, hide it witharia-hidden="true"so the id is not announced twice.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/components/providers/organizations/org-account-tree-item.tsx` around lines 82 - 101, Update TruncatedId so the canonical value is exposed through valid text content rather than aria-label on the plain span. When shortened is displayed, render the canonical value in an accessible element and mark the shortened presentation with aria-hidden="true" to prevent duplicate announcements; preserve the tooltip and visual truncation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/app/`(prowler)/providers/providers-page.integration.test.tsx:
- Around line 138-140: Export AZURE_ORG_NAME and AZURE_GROUP_NODE_ID from
organizations.fixtures.ts, then import both in the existing fixture import block
of the integration test. Remove the local literal declarations so
waitForOrganizationRow and waitForNodeDelete use the fixture-owned values.
In `@ui/components/providers/organizations/hooks/org-setup-strategy.test.ts`:
- Around line 74-87: Strengthen the test in the org flow
`it.each(ORG_FLOW_TYPES)` case by also asserting that the returned `message`
does not contain the server-provided `"Hierarchy too deep."` text, while
retaining the existing curated-wording assertion.
In `@ui/components/providers/organizations/hooks/org-setup-strategy.ts`:
- Around line 404-421: Update the doc comment above bindOrgSetupStrategy to
describe the default arm as a compile-time exhaustiveness check using the never
assignment, rather than implying it produces a runtime message or names an
offending organization type.
In `@ui/components/providers/workflow/forms/connect-account-form.tsx`:
- Around line 398-409: Update the Azure method screen branch in the provider
form, identified by prevStep === 2, providerType === "azure", and method ===
null, to provide an explicit tour anchor or tour-visibility handling for its
rendered content. Keep the existing ProviderTitleDocs and AzureMethodSelector
behavior unchanged, and align the added handling with the existing showUidForm
and tour-aware state used by the Next button.
---
Outside diff comments:
In `@ui/components/providers/organizations/org-account-tree-item.tsx`:
- Around line 82-101: Update TruncatedId so the canonical value is exposed
through valid text content rather than aria-label on the plain span. When
shortened is displayed, render the canonical value in an accessible element and
mark the shortened presentation with aria-hidden="true" to prevent duplicate
announcements; preserve the tooltip and visual truncation behavior.
In `@ui/lib/organizations.test.ts`:
- Around line 120-136: Add shortenNodeId to the organizations test import and
add coverage for its ARM prefix matching, including case-insensitive input, plus
assertions that AWS OU IDs and GCP folder references return undefined. Keep the
existing canonical-ID behavior validated for unsupported formats.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a036de62-0e0c-4b0e-a039-0f295a882cee
📒 Files selected for processing (34)
ui/__tests__/msw/handlers/organizations.fixtures.tsui/__tests__/msw/handlers/organizations.tsui/actions/organizations/organizations.adapter.tsui/actions/organizations/organizations.test.tsui/actions/organizations/organizations.tsui/app/(prowler)/providers/providers-page.harness.tsxui/app/(prowler)/providers/providers-page.integration.test.tsxui/changelog.d/azure-management-group-onboarding.added.mdui/changelog.d/org-hierarchy-depth-copy.fixed.mdui/components/providers/organizations/azure-method-selector.tsxui/components/providers/organizations/azure-org-setup-form.tsxui/components/providers/organizations/gcp-org-setup-form.tsxui/components/providers/organizations/hooks/org-setup-strategy.test.tsui/components/providers/organizations/hooks/org-setup-strategy.tsui/components/providers/organizations/hooks/use-org-setup-submission.test.tsui/components/providers/organizations/hooks/use-org-setup-submission.tsui/components/providers/organizations/org-account-tree-item.tsxui/components/providers/organizations/org-setup-form.tsxui/components/providers/organizations/org-terminology.tsui/components/providers/table/data-table-row-actions.test.tsxui/components/providers/wizard/hooks/use-provider-wizard-controller.tsui/components/providers/wizard/provider-wizard-modal.tsxui/components/providers/wizard/provider-wizard-modal.utils.test.tsui/components/providers/wizard/provider-wizard-modal.utils.tsui/components/providers/workflow/forms/connect-account-form.tsxui/lib/cloud-upgrade.test.tsui/lib/cloud-upgrade.tsui/lib/external-urls.tsui/lib/organizations.test.tsui/lib/organizations.tsui/store/organizations/store.test.tsui/tests/providers/providers-page.tsui/types/cloud-upgrade.tsui/types/organizations.ts
pfe-nazaries
force-pushed
the
feature/azure-org-onboarding-ui
branch
from
August 10, 2026 10:01
a964d88 to
ae4e624
Compare
Contributor
Author
CodeRabbit Autofix Review CompleteReviewed 4 feedback items, applied 2. Branch was also rebased onto the updated Applied
Not applied
Files modified
Typecheck, ESLint, Prettier and the related unit tests pass on |
Contributor
Author
Organization name contract — alignedThanks, this was a real inconsistency. All three of your points checked out, though one had moved: Two findings shaped the fix:
So the contract is now the honest one, and identical in both directions:
Commit |
pfe-nazaries
force-pushed
the
feature/azure-org-onboarding-ui
branch
from
August 10, 2026 13:18
12abaed to
3307415
Compare
pfe-nazaries
force-pushed
the
feature/azure-org-onboarding-ui
branch
from
August 11, 2026 07:29
3307415 to
4a8c07d
Compare
Alan-TheGentleman
previously approved these changes
Aug 11, 2026
Alan-TheGentleman
left a comment
Alan-TheGentleman
left a comment
Contributor
There was a problem hiding this comment.
The organization name contract is now consistent across creation and renaming. Approved.
pfe-nazaries
force-pushed
the
feature/azure-org-onboarding-ui
branch
from
August 11, 2026 13:41
4a8c07d to
81fd7b8
Compare
pfe-nazaries
force-pushed
the
feature/azure-org-onboarding-ui
branch
from
August 11, 2026 13:42
81fd7b8 to
78f0839
Compare
Alan-TheGentleman
previously approved these changes
Aug 11, 2026
Base automatically changed from
feature/azure-org-suite-consolidation
to
master
August 11, 2026 16:50
pfe-nazaries
force-pushed
the
feature/azure-org-onboarding-ui
branch
from
August 11, 2026 16:50
78f0839 to
6e98a1f
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
- Add Azure discovery, hierarchy, secret and apply payload types - Enable the Azure organizations flow and the management-group node kind - Discriminate organization secret payloads on organization type - Extend the setup strategy with the root external id and Azure fields - Leave the per-type gaps the compiler now enumerates to the Azure arms - Record the sharp install in the UI dependency log
- Add the Azure organizations method selector and setup form - Map management-group discovery and apply the selected subscriptions - Show the cloud upgrade upsell for the Azure method outside Prowler Cloud - Cover the flow in the consolidated providers page integration suite - Drop the dead organization root write seam
- Tell Azure users about Management Groups, not folders they cannot have - Move per-provider discovery copy onto the setup strategy arms - Require every organization type to bring its own shared-code wording - Cover the shared code across all onboarding flows
- Show the Management Group name where the ARM id is all shared prefix - Keep the canonical resource id in the tooltip and accessible name - Leave AWS and GCP identifiers rendered in full - Assert what the id column reads, not only the uid behind it
- Read the discovery root from `root_management_group`, the key the API actually sends, instead of `root` — the previous name threw on ingest - Replace the dead `azure_management_group_not_found` copy with `azure_root_management_group_not_found` and curate the emitted codes that had no wording - Correct the Azure blocked-reason names to the conflict vocabulary the API shares with GCP, and cover a subscription blocked only by state - Casefold the tenant ID so find-or-create matches the canonical external id the API stores
- Add never-assigning default arms to the apply payload and strategy switches - Reach the setup forms' own node with a ref instead of getElementById - Expose the canonical container id via aria-label so the harness drops its regex - Restore the dependency log to its committed state
- Re-drop the wizard modal comments removed in 102d2b5 - Trim the apply-payload guard note to its non-obvious why
- Validate the tenant and client IDs with z.guid() instead of z.uuid() - z.uuid() enforces the RFC-9562 version and variant nibbles, so it rejected real Azure identifiers while accepting the nil UUID
- Give the id span role="img" so its aria-label is honoured - ARIA prohibits naming a bare span, letting screen readers fall back to the shortened text and lose the canonical id - Point the harness's inert-note lookup at the icon, now that the id column carries the role as well
- Remove the EDIT_NAME branch, its save handler and the state it owned - Nothing produces that intent: the row actions only ever pass EDIT_CREDENTIALS, and renaming happens in the inline modal
- State the identifier the name falls back to, not a provider-side name: the organization is created before discovery, so no such name is known - Apply the same fallback when renaming, which previously rejected a blank - Centralize the hint so the three setup forms cannot word it differently
- Drop comments that restate the code or the test title they sit under - Keep each rationale where it belongs instead of repeating it per call site - Reattach the AWS hierarchy fixture doc to the fixture it documents
pfe-nazaries
force-pushed
the
feature/azure-org-onboarding-ui
branch
from
August 11, 2026 16:50
6e98a1f to
ea2f79e
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/__tests__/msw/handlers/organizations.fixtures.ts`:
- Around line 737-741: Update the documentation comment describing the fresh
Azure organization onboarding fixture to state that three subscriptions are
blocked, matching the result produced by buildAzureDiscoveryResult; leave the
readiness and management-group details unchanged.
In `@ui/app/`(prowler)/providers/providers-page.integration.test.tsx:
- Around line 155-162: Refactor OrganizationSecretRequestBody by extracting its
nested data and attributes object types into dedicated interfaces, then
reference those interfaces from the parent interface instead of using inline
nested objects. Follow the existing RenameRequestBody pattern and preserve the
optional secret_type and secret properties.
In `@ui/components/providers/organizations/azure-org-setup-form.tsx`:
- Around line 145-171: Update the Azure organization setup submit flow to handle
ORG_WIZARD_INTENT.EDIT_NAME by calling updateOrganizationName with the existing
organization ID and the new name instead of advancing to ORG_SETUP_PHASE.ACCESS.
Pass the modal’s onClose callback to the update flow and invoke it only after
the update succeeds; preserve the existing behavior for other intents.
In `@ui/components/providers/table/data-table-row-actions.tsx`:
- Around line 228-236: Normalize the name value in the edit-name flow around the
validate callback and onSave handler: trim whitespace before validation so
whitespace-only input is rejected, and trim before choosing between the entered
name and nameFallback so whitespace-only input uses the fallback. Preserve the
existing required-message and updateOrganizationName behavior for non-empty
names.
In `@ui/lib/external-urls.ts`:
- Around line 17-18: Do not expose AZURE_ORGANIZATIONS until the linked Azure
management-group documentation reflects the released onboarding flow; either
update the documentation content first or gate/remove this URL entry until that
update is available.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 48ad0566-9f61-4c45-9f41-aee9eddd734a
📒 Files selected for processing (35)
ui/__tests__/msw/handlers/organizations.fixtures.tsui/__tests__/msw/handlers/organizations.tsui/actions/organizations/organizations.adapter.tsui/actions/organizations/organizations.test.tsui/actions/organizations/organizations.tsui/app/(prowler)/providers/providers-page.harness.tsxui/app/(prowler)/providers/providers-page.integration.test.tsxui/changelog.d/azure-management-group-onboarding.added.mdui/changelog.d/org-hierarchy-depth-copy.fixed.mdui/components/providers/organizations/azure-method-selector.tsxui/components/providers/organizations/azure-org-setup-form.tsxui/components/providers/organizations/gcp-org-setup-form.tsxui/components/providers/organizations/hooks/org-setup-strategy.test.tsui/components/providers/organizations/hooks/org-setup-strategy.tsui/components/providers/organizations/hooks/use-org-setup-submission.test.tsui/components/providers/organizations/hooks/use-org-setup-submission.tsui/components/providers/organizations/org-account-tree-item.tsxui/components/providers/organizations/org-setup-form.tsxui/components/providers/organizations/org-terminology.tsui/components/providers/table/data-table-row-actions.test.tsxui/components/providers/table/data-table-row-actions.tsxui/components/providers/wizard/hooks/use-provider-wizard-controller.tsui/components/providers/wizard/provider-wizard-modal.tsxui/components/providers/wizard/provider-wizard-modal.utils.test.tsui/components/providers/wizard/provider-wizard-modal.utils.tsui/components/providers/workflow/forms/connect-account-form.tsxui/lib/cloud-upgrade.test.tsui/lib/cloud-upgrade.tsui/lib/external-urls.tsui/lib/organizations.test.tsui/lib/organizations.tsui/store/organizations/store.test.tsui/tests/providers/providers-page.tsui/types/cloud-upgrade.tsui/types/organizations.ts
- Trim the organization name before validating and before the fallback - Flatten the secret request body interface in the providers suite - Correct the blocked-subscription count in the Azure fixture comment
Contributor
Author
CodeRabbit autofix — 3 applied, 2 dismissedReviewed 5 unresolved CodeRabbit threads. All 5 are now resolved. Applied in
Dismissed, with reasons:
|
jfagoagas
approved these changes
Aug 12, 2026
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
4 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.
Context
Azure customers with many subscriptions can only onboard one subscription at a time today, each with its own service-principal credential, while AWS Organizations and GCP Organizations already have one-click multi-account onboarding. This closes that parity gap: Azure Management Group onboarding, tracked on the roadmap for 2026-Q3.
The GCP effort (merged 2026-07-31) deliberately left a provider-agnostic organization contract behind — strategy seam, discriminated unions, exhaustive
satisfies Record<OrgFlowType,…>tables, shared wizard lifecycle, browser-mode harness, MSW fixtures. Azure is therefore a compile-enumerated extension of that contract rather than a rebuild: addingAZUREtoORG_FLOW_TYPESmakes the type checker list every remaining gap.This is layer 3 of the Azure stack, on top of #12383.
Jira: PROWLER-2365 (epic PROWLER-2364). The API side is PROWLER-2366.
Description
New Azure-specific pieces, all
azure-*-prefixed, plugged into existing seams:azure-method-selector.tsx— single subscription vs Management Group, with the cloud-upgrade upsell outside Prowler Cloud.azure-org-setup-form.tsx— Entra tenant ID + optional name, then service-principal credentials.azureOrgSetupStrategy— external id, secret payload, secret-error mapping, discovery ingest, auth-failure copy.mapAzureDiscovery+ the apply payload arm, mappingroot/management_groups/subscriptionsonto the shared tree.NODE_KINDfor management groups, badge/docs/upsell/terminology entries.Everything else is reused unchanged: the submission chain (find-or-create org → secret → discovery → poll → ingest), tree selection, apply, connection tests, launch/schedule, re-entry via Update Credentials, kind-aware delete, degraded-hierarchy fallback.
Three points from the API contract are worth calling out, because they are simpler than an early reading of it suggests:
root_external_idwrite seam is dead code and is removed here; the read path stays.{client_id, client_secret}only; the tenant comes from the organization.error_messagebeside the machineerrorcode. Failure copy resolves curated →error_message→ auth fallback, so a code the API adds later still says something useful.Testing follows the integration-first policy: the Azure cases were written red against a frozen type frontier, then implemented. They live as a new describe group inside the consolidated
providers-page.integration.test.tsx(the "1 integration file = 1 real page" convention introduced in #12383), one case per spec scenario — happy path, subscription nesting, alias prefill, inert and blocked rows, apply payload shape, both credential-replace warnings, discovery failure + retry, launch outcomes, OSS gating, kind-aware delete. No new unit tests except where internals warrant them.One real defect surfaced while turning them green: the harness read a row's uid from the whole row's
textContent, which glues the uid column to the name column — so the management-group pattern swallowed the display name (…/managementGroups/archivedArchived). It now matches per text node, which also removes the same latent trap for AWS OU ids.Steps to review
types/organizations.tsfirst — it is the contract the rest of the diff is forced into.cd ui && pnpm exec vitest run --project integration→ 61/61. Run the two projects separately: running them concurrently starves the browser workers and times out unrelated suites.cd ui && pnpm exec vitest run --project unit→ 2804/2804.pnpm run typecheckandpnpm run lint→ clean.pull_requeston basemaster/v5.*, so only the bottom of a stack is gated. The full set runs once this becomes base-master.Checklist
<component>/changelog.d/. —ui/changelog.d/azure-management-group-onboarding.added.md.UI
ui/changelog.d/.License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Summary by CodeRabbit