Skip to content

feat(scanner): add enterprise App Registration and Managed Identity rules#197

Merged
Vishnu2707 merged 2 commits into
devfrom
feat/193-app-reg-managed-identity
Jul 18, 2026
Merged

feat(scanner): add enterprise App Registration and Managed Identity rules#197
Vishnu2707 merged 2 commits into
devfrom
feat/193-app-reg-managed-identity

Conversation

@TFT444

@TFT444 TFT444 commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

Adds a cohesive enterprise identity security pack for Microsoft Entra App Registrations and Azure Managed Identities, while consolidating App Registration inventory so all related rules share one cached, paginated Graph request per scan.

Rules

Rule Severity Detection
AZ-IDN-010 MEDIUM App Registration has no owner
AZ-IDN-011 HIGH Non-loopback HTTP redirect URI
AZ-IDN-012 MEDIUM OAuth implicit token issuance enabled
AZ-IDN-013 MEDIUM Password credentials present
AZ-IDN-014 HIGH Multi-tenant app lacks full application-instance property lock
AZ-IDN-015 HIGH Managed identity has Owner or Contributor at subscription scope

Existing rule improvement

AZ-IDN-006 now reuses AzureClient.get_applications() instead of performing its own Graph request. The accessor:

  • follows @odata.nextLink;
  • caches successful, empty, and failed inventory states;
  • expands only minimal owner IDs;
  • requests only security-relevant application properties;
  • returns None on Graph failure so rules cannot mistake an incomplete scan for compliance.

Managed Identity detection uses Graph's immutable servicePrincipalType=ManagedIdentity filter and correlates principal IDs with cached subscription RBAC assignments. Privileged roles are matched by immutable built-in role-definition GUIDs and exact subscription scope.

Data minimization

Findings do not include:

  • tokens or credential values;
  • credential key IDs or hints;
  • complete redirect URIs;
  • owner names, email addresses, or user principal names.

Only counts, boolean states, role names, principal IDs, and required resource identifiers are retained.

Assignment restriction decision

The proposed AZ-IDN-016 resource-provider assignment-restriction rule is intentionally deferred. Microsoft documents the new portal feature, but the current public Managed Identity REST response reliably exposes isolationScope, not the configured resource-provider restriction collection. This PR does not infer or invent an undocumented field.

Permissions

Read-only scanning requires:

  • Microsoft Graph application permission Application.Read.All;
  • Azure action Microsoft.Authorization/roleAssignments/read.

No Graph write permission is requested by the scanner.

Compliance

Mappings were added for the repository's NIST CSF 1.1, ISO/IEC 27001:2013, and SOC 2 schemas. The current CIS Azure Foundations 2.0.0 file has no direct controls for these checks, so mappings are explicitly numbered TBD-IDN-* placeholders rather than claiming unsupported CIS coverage.

Testing

  • 409 tests passed from a clean checkout
  • 3 existing environment-dependent tests skipped
  • 58 focused identity tests passed
  • Pagination and per-scan caching tested
  • Graph and Azure RBAC failures tested as indeterminate states
  • Loopback IPv4, IPv6, localhost, HTTPS, and native redirect cases tested
  • Exact subscription versus resource-group scope tested
  • Ruff lint and format checks passed
  • Bash syntax checks passed for all playbooks
  • Bandit passed at the CI threshold
  • No new dependency introduced
  • Live tenant validation (requires an approved non-production Entra/Azure environment)

Related issue

Closes #193

@github-actions

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@TFT444
TFT444 requested review from m-khan-97 and removed request for SHAURYAKSHARMA24, parthrohit22 and ritiksah141 July 15, 2026 12:54
@TFT444 TFT444 self-assigned this Jul 16, 2026

@m-khan-97 m-khan-97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed thoroughly — fetched the branch and ran the tests locally (34 passed), plus checked the redirect-URI classifier against edge cases directly.

Strong work:

  • get_service_principals() refactor preserves behavior. It now delegates to the cached get_subscription_role_assignments(), and still returns [] on failure (when assignments is None), so no regression for existing callers. The _UNSET sentinel correctly distinguishes "not yet fetched" from "fetched and failed (None)".
  • AZ-IDN-006 correctly reuses the shared get_applications() instead of its own inline Graph fetch, and handles the None (inventory-unavailable) case rather than silently treating it as "no findings."
  • Redirect-URI logic (AZ-IDN-011) is correct. Verified http://[::1], http://127.0.0.1:8080, and http://localhost are all treated as loopback (not flagged), while http://evil.com is flagged and msalclient:// (non-http scheme) is ignored. ipaddress.ip_address() on the bracket-stripped IPv6 host works as intended.
  • Data minimization is enforced and tested — findings carry counts/booleans only, and the tests explicitly assert the raw URI and a sentinel keyId never appear in metadata. Good follow-through on the #178 lesson.
  • Every rule has a matching confirmation-gated playbook; the destructive ones (secret deletion, implicit-grant disable) are print-only or APPLY-gated.

Minor cosmetic note, non-blocking: the new rules use TBD-IDN-0xx as the CIS control_id, while the existing AKS rules used N/A-AKS-006. Since the descriptions say these genuinely aren't in CIS Foundations 2.0.0, N/A- reads as more accurate than TBD- (which implies "coming soon"). Not worth a round-trip.

Approving.

@Vishnu2707
Vishnu2707 merged commit 4669dc6 into dev Jul 18, 2026
19 checks passed
@Vishnu2707
Vishnu2707 deleted the feat/193-app-reg-managed-identity branch July 18, 2026 00:35
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.

feat(scanner): propose enterprise App Registration and Managed Identity security pack

3 participants