diff --git a/.claude/skills/remediation-tool/scripts/onboard-tenant.sh b/.claude/skills/remediation-tool/scripts/onboard-tenant.sh index ef2fab1..589684d 100755 --- a/.claude/skills/remediation-tool/scripts/onboard-tenant.sh +++ b/.claude/skills/remediation-tool/scripts/onboard-tenant.sh @@ -324,11 +324,12 @@ consent_app() { } # ── Helper: check if directory role already assigned ───────────────────────── -# TODO(howard): This only checks roleAssignments (direct/permanent). PIM-managed -# assignments live in roleAssignmentSchedules and won't be found here, causing -# noisy-but-harmless "MISSING -> ASSIGNING" output that hits the Conflict fallback. -# Fix: also query /roleManagement/directory/roleAssignmentSchedules?$filter=principalId eq '...' -# and return true if either query finds the role. Reference: Howard's note 2026-04-29. +# NOTE: The "MISSING -> ASSIGNING" noise was NOT PIM, as previously suspected — the +# root cause was an unencoded space in the $filter (now %20-encoded), which made Graph +# return empty/error and this function always return false. The ACG tenant has no Entra +# ID P2, so PIM is not a factor here. The dual-query idea (also checking +# /roleManagement/directory/roleAssignmentSchedules) remains valid ONLY for P2 tenants +# where roles can be PIM-managed; return true if either query finds the role. role_assigned() { local token="$1" local sp_oid="$2" @@ -336,7 +337,7 @@ role_assigned() { local resp resp=$(curl -s --max-time 15 \ -H "Authorization: Bearer $token" \ - "https://graph.microsoft.com/v1.0/roleManagement/directory/roleAssignments?\$filter=principalId eq '${sp_oid}'") + "https://graph.microsoft.com/v1.0/roleManagement/directory/roleAssignments?\$filter=principalId%20eq%20'${sp_oid}'") echo "$resp" | jq --arg rid "$role_id" \ '[.value[] | select(.roleDefinitionId == $rid)] | length > 0' }