Enhance error handling in RemoveServicePrincipalRoleAssignment methods and update return types to indicate success or failure

[gautamdsheth]

Type

• Bug Fix
• New Feature
• Sample

Related Issues?

Fixes #5342

What is in this Pull Request ?

Fixes app role error and update docs

[Copilot]

Pull request overview

This PR addresses Entra ID service principal app role removal reliability (Fixes #5342) by improving role-assignment matching/removal behavior and surfacing failure to callers, and it updates Microsoft Teams channel cmdlet permission metadata and docs to reflect additional Graph permissions used by some scenarios.

Changes:

• Update ServicePrincipalUtility.RemoveServicePrincipalRoleAssignment overloads to return bool indicating whether any assignments were removed, and broaden matching for role removal by name (case-insensitive and GUID support).
• Update Remove-PnPEntraIDServicePrincipalAssignedAppRole to throw when a requested assignment is not found/removed.
• Adjust Teams channel cmdlet permission attributes and enhance Add-PnPTeamsChannel documentation for -OwnerUPN scenarios.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/Commands/Utilities/ServicePrincipalUtility.cs	Return success/failure from role-assignment removal and enhance role matching logic.
src/Commands/Apps/RemoveEntraIDServicePrincipalAssignedAppRole.cs	Throw when requested app role assignment removal does not occur.
src/Commands/Base/PipeBinds/ServicePrincipalAvailableAppRoleBind.cs	Add a stable Value and ToString() to ensure consistent role-name/ID string usage.
src/Commands/Teams/AddTeamsChannel.cs	Add Graph permission attributes related to channel membership operations used by some channel types.
src/Commands/Teams/AddTeamsChannelUser.cs	Update permission attributes to include delegated-or-application and app-only alternatives.
documentation/Add-PnPTeamsChannel.md	Document additional required permissions when using -OwnerUPN and add related Graph links.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

src/Commands/Utilities/ServicePrincipalUtility.cs:269

• GetServicePrincipalAppRoleAssignmentsByServicePrincipalObjectId() can return null (it swallows exceptions and returns null), which will cause a NullReferenceException here when iterating. This is especially problematic for a method intended to improve error handling; consider throwing a clearer exception when assignments cannot be retrieved.

        public static void RemoveServicePrincipalRoleAssignment(ApiRequestHelper requestHelper, AzureADServicePrincipal principalToRemoveRoleFrom)
        {
            var assignments = GetServicePrincipalAppRoleAssignmentsByServicePrincipalObjectId(requestHelper, principalToRemoveRoleFrom.Id);
            foreach (var assignment in assignments)
            {
                requestHelper.Delete($"v1.0/servicePrincipals/{principalToRemoveRoleFrom.Id}/appRoleAssignments/{assignment.Id}");
            }
        }

src/Commands/Utilities/ServicePrincipalUtility.cs:344

• The parameter name appRoleAssignmenToRemove is misspelled (missing the 't' in 'Assignment'), and the method will currently throw a NullReferenceException if called with a null assignment. Renaming the parameter and adding an explicit null check will improve clarity and error handling without impacting call sites using positional arguments.

        /// <param name="appRoleAssignmenToRemove">The app role assignment to remove from the service principal</param>
        public static void RemoveServicePrincipalRoleAssignment(ApiRequestHelper requestHelper, AzureADServicePrincipalAppRoleAssignment appRoleAssignmenToRemove)
        {
            requestHelper.Delete( $"v1.0/servicePrincipals/{appRoleAssignmenToRemove.PrincipalId}/appRoleAssignments/{appRoleAssignmenToRemove.Id}");
        }