ENG-2766: Add isDisabled and web to Entra Graph $select fields#7734
ENG-2766: Add isDisabled and web to Entra Graph $select fields#7734dsill-ethyca merged 4 commits intomainfrom
Conversation
isDisabled is needed to detect deactivated apps (maps to INACTIVE app_status). web is needed for domain extraction via homePageUrl. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR adds two fields —
Confidence Score: 5/5
Important Files Changed
Reviews (1): Last reviewed commit: "Fix ruff format: combine string literals..." | Re-trigger Greptile |
There was a problem hiding this comment.
The change itself is minimal and well-structured — the existing tests import APPLICATIONS_SELECT by reference so they'll pick up the new fields without any changes needed there.
One concern worth investigating: isDisabled is a property on the servicePrincipal resource in Microsoft Graph v1.0, not the application resource. Querying /v1.0/applications with $select=isDisabled will likely result in that field being silently omitted from responses (Graph ignores unrecognized $select fields rather than erroring), meaning the downstream inactive-app filtering in fidesplus would silently do nothing.
The standard way to detect disabled app registrations in Entra is to query the corresponding servicePrincipal and check accountEnabled. If the author has confirmed isDisabled is actually populated in responses from this endpoint (e.g. via a real Entra tenant test), this concern is resolved — otherwise it may need a follow-up to use the correct field/endpoint.
The web field addition looks correct — it is a valid top-level property on the application resource and contains homePageUrl and redirectUris as described.
| APPLICATIONS_SELECT = "id,appId,displayName,createdDateTime,description,signInAudience" | ||
| # $select fields for IDP monitor discovery | ||
| APPLICATIONS_SELECT = ( | ||
| "id,appId,displayName,createdDateTime,description,signInAudience,isDisabled,web" |
There was a problem hiding this comment.
isDisabled is a property on the servicePrincipal resource in Microsoft Graph v1.0, not the application resource. The /v1.0/applications endpoint returns application objects, which don't have a top-level isDisabled field.
If you include an unrecognized $select field, Graph silently omits it from the response (no error), so this won't break anything — but isDisabled will always be absent from the returned objects and the downstream null-check / inactive filtering in fidesplus won't work as intended.
To check disabled state for app registrations you'd typically query the corresponding servicePrincipal via /v1.0/servicePrincipals?$filter=appId eq '<appId>'&$select=accountEnabled (the accountEnabled field on servicePrincipal is the canonical way to detect disabled apps in Entra).
Worth confirming you're seeing isDisabled populated in actual test runs against a real Entra tenant, or that this is a beta/preview field that works on this endpoint.
There was a problem hiding this comment.
FYI isDisabled select works on an actual graph request
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Description of Changes
Adds
isDisabledandwebto the Microsoft Graph$selectquery for the Entra/applicationsendpoint.isDisabled— Needed to detect deactivated app registrations. Maps toINACTIVEapp_status in fidesplus, enabling the IDP monitor to filter out disabled apps (matching Okta's behavior).web— ContainshomePageUrlandredirectUrisused by the Entra domain extractor to resolve vendor domains for Compass matching and Brandfetch logos. Without this field, domain extraction only works via LLM fallback.Related PRs
Code Changes
src/fides/api/service/connectors/entra_http_client.py— UpdatedAPPLICATIONS_SELECTconstantSteps to Confirm
isDisabled: true) are filtered outweb.homePageUrlhave domains extractedPre-Merge Checklist
🤖 Generated with Claude Code