Get-AzADGroupMember should return better error message if group does not exist

jikuja commented 1 week ago

Description

Cmdlet does not check if Get-ADGroup query returns group(s): https://github.com/Azure/azure-powershell/blob/main/src/Resources/MSGraph.Autorest/custom/Get-AzADGroupMember.ps1#L132-L137

The current error message(Get-AzADGroupMember: Cannot bind argument to parameter 'GroupId' because it is an empty string.) is really misleading because used does not supply GroupId.

Cmdlet should check that Get-AzADGroup returns exactly on group or error with human-readable error message. This check should be done on all Cmdlets using Graph API to solve Displayname or any other filtering and remaining logic assumes that Get-AzADGroup/User/serviceprincipal returns exactly one item.

Issue script & Debug output

$DebugPreference = 'Continue'
DEBUG: 11:41:53 AM - [ConfigManager] Got nothing from [DisplaySecretsWarning], Module = [], Cmdlet = []. Returning default value [True].
DEBUG: 11:41:53 AM - GetAzureRMContextCommand begin processing with ParameterSet 'GetSingleContext'.
DEBUG: 11:41:53 AM - [ConfigManager] Got [False] from [DisplayBreakingChangeWarning], Module = [], Cmdlet = [].
DEBUG: 11:41:53 AM - [ConfigManager] Got nothing from [DisplaySecretsWarning], Module = [], Cmdlet = []. Returning default value [True].
DEBUG: 11:41:53 AM - [ConfigManager] Got nothing from [DisplayRegionIdentified], Module = [], Cmdlet = []. Returning default value [True].
DEBUG: 11:41:53 AM - [ConfigManager] Got nothing from [CheckForUpgrade], Module = [], Cmdlet = []. Returning default value [True].
DEBUG: AzureQoSEvent:  Module: Az.Accounts:3.0.4; CommandName: Get-AzContext; PSVersion: 7.4.4; IsSuccess: True; Duration: 00:00:00.0015950; SanitizeDuration: 00:00:00.0003214
DEBUG: 11:41:53 AM - [ConfigManager] Got nothing from [EnableDataCollection], Module = [], Cmdlet = []. Returning default value [True].
DEBUG: 11:41:53 AM - GetAzureRMContextCommand end processing.
 janne.kujanpaa   luvitusprojekti     Get-AzADGroupMember -GroupDisplayName entra1-dw-reader-dev
DEBUG: 11:42:18 AM - [ConfigManager] Got nothing from [DisplaySecretsWarning], Module = [], Cmdlet = []. Returning default value [True].
WARNING: This cmdlet is using a preview API version and is subject to breaking change in a future release.
DEBUG: [CmdletBeginProcessing]: Starting command
DEBUG: CmdletBeginProcessing: 
DEBUG: CmdletProcessRecordStart: 
DEBUG: Client side pagination is enabled for this cmdlet
DEBUG: CmdletGetPipeline: 
DEBUG: CmdletBeforeAPICall: 
DEBUG: URLCreated: /groups?$filter=displayName%20eq%20%27entra1-dw-reader-dev%27
DEBUG: RequestCreated: /v1.0/groups?$filter=displayName%20eq%20%27entra1-dw-reader-dev%27
DEBUG: HeaderParametersAdded: 
DEBUG: ============================ HTTP REQUEST ============================

HTTP Method:
GET

Absolute Uri:
https://graph.microsoft.com/v1.0/groups?$filter=displayName eq %27entra1-dw-reader-dev%27

Headers:
x-ms-client-request-id        : 7a71459f-ab76-4f11-ae08-ca7bdee8f316
CommandName                   : Az.MSGraph.internal\Get-AzAdGroup
FullCommandName               : Get-AzADGroup_List
ParameterSetName              : __AllParameterSets
User-Agent                    : AzurePowershell/v12.3.0,PSVersion/v7.4.4,Az.MSGraph/7.4.0

Body:

DEBUG: BeforeCall: 
DEBUG: 11:42:18 AM - [ConfigManager] Got nothing from [DisableInstanceDiscovery], Module = [], Cmdlet = []. Returning default value [False].
DEBUG: ============================ HTTP RESPONSE ============================

Status Code:
OK

Headers:
Cache-Control                 : no-cache
Transfer-Encoding             : chunked
Strict-Transport-Security     : max-age=31536000
request-id                    : ca8da28a-2646-452a-8b0f-bb2f27635f01
client-request-id             : ca8da28a-2646-452a-8b0f-bb2f27635f01
x-ms-ags-diagnostic           : {"ServerInfo":{"DataCenter":"Sweden Central","Slice":"E","Ring":"3","ScaleUnit":"002","RoleInstance":"GV3PEPF00000B3B"}}
x-ms-resource-unit            : 1
OData-Version                 : 4.0
Date                          : Tue, 08 Oct 2024 08:42:18 GMT

Body:
{
  "@odata.context": "https://graph.microsoft.com/v1.0/$metadata#groups",
  "value": []
}

DEBUG: ResponseCreated: 
DEBUG: BeforeResponseDispatch: 
DEBUG: Finally: 
DEBUG: CmdletAfterAPICall: 
DEBUG: [CmdletProcessRecordAsyncEnd]: Finish HTTP process
DEBUG: CmdletProcessRecordAsyncEnd: 
DEBUG: CmdletProcessRecordEnd: 
Get-AzADGroupMember: Cannot bind argument to parameter 'GroupId' because it is an empty string.
DEBUG: 11:42:19 AM - [ConfigManager] Got nothing from [DisplaySecretsWarning], Module = [], Cmdlet = []. Returning default value [True].
DEBUG: 11:42:19 AM - GetAzureRMContextCommand begin processing with ParameterSet 'GetSingleContext'.
DEBUG: 11:42:19 AM - [ConfigManager] Got [False] from [DisplayBreakingChangeWarning], Module = [], Cmdlet = [].
DEBUG: 11:42:19 AM - [ConfigManager] Got nothing from [DisplaySecretsWarning], Module = [], Cmdlet = []. Returning default value [True].
DEBUG: 11:42:19 AM - [ConfigManager] Got nothing from [DisplayRegionIdentified], Module = [], Cmdlet = []. Returning default value [True].
DEBUG: 11:42:19 AM - [ConfigManager] Got nothing from [CheckForUpgrade], Module = [], Cmdlet = []. Returning default value [True].
DEBUG: AzureQoSEvent:  Module: Az.Accounts:3.0.4; CommandName: Get-AzContext; PSVersion: 7.4.4; IsSuccess: True; Duration: 00:00:00.0025919; SanitizeDuration: 00:00:00.0001428
DEBUG: 11:42:19 AM - [ConfigManager] Got nothing from [EnableDataCollection], Module = [], Cmdlet = []. Returning default value [True].
DEBUG: 11:42:19 AM - GetAzureRMContextCommand end processing.

Environment data

Name                           Value
----                           -----
PSVersion                      7.4.4
PSEdition                      Core
GitCommitId                    7.4.4
OS                             Darwin 22.6.0 Darwin Kernel Version 22.6.0: Mon Jun 24 01:21:41 PDT 2024; root:xnu-8796.141.3.706.2~1/RELEASE_…
Platform                       Unix
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Module versions

ModuleType Version    PreRelease Name                                ExportedCommands
---------- -------    ---------- ----                                ----------------
Script     3.0.4                 Az.Accounts                         {Add-AzEnvironment, Clear-AzConfig, Clear-AzContext, Clear-AzDefault…}
Script     7.4.0                 Az.Resources                        {Export-AzResourceGroup, Export-AzTemplateSpec, Get-AzDenyAssignment, Ge…
Script     1.1.2                 Az.Tools.Predictor                  {Disable-AzPredictor, Enable-AzPredictor, Open-AzPredictorSurvey, Send-A…

Error output

HistoryId: 2

Message        : Cannot bind argument to parameter 'GroupId' because it is an empty string.
StackTrace     :    at System.Management.Automation.ExceptionHandlingOps.CheckActionPreference(FunctionContext funcContext, Exception exception)
                    at System.Management.Automation.Interpreter.ActionCallInstruction`2.Run(InterpretedFrame frame)
                    at System.Management.Automation.Interpreter.EnterTryCatchFinallyInstruction.Run(InterpretedFrame frame)
                    at System.Management.Automation.Interpreter.EnterTryCatchFinallyInstruction.Run(InterpretedFrame frame)
                    at System.Management.Automation.Interpreter.Interpreter.Run(InterpretedFrame frame)
                    at System.Management.Automation.Interpreter.LightLambda.RunVoid1[T0](T0 arg0)
                    at System.Management.Automation.PSScriptCmdlet.RunClause(Action`1 clause, Object dollarUnderbar, Object inputToProcess)
                    at System.Management.Automation.PSScriptCmdlet.DoProcessRecord()
                    at System.Management.Automation.CommandProcessor.ProcessRecord()
Exception      : System.Management.Automation.ParameterBindingValidationException
InvocationInfo : {Get-AzADGroupMember}
Line           :         Az.MSGraph.internal\Get-AzADGroupMember @PSBoundParameters

Position       : At /Users/janne.kujanpaa/.local/share/powershell/Modules/Az.Resources/7.4.0/MSGraph.Autorest/custom/Get-AzADGroupMember.ps1:140 char:49
                 +         Az.MSGraph.internal\Get-AzADGroupMember @PSBoundParameters
                 +                                                 ~~~~~~~~~~~~~~~~~~
HistoryId      : 2

isra-fel commented 1 week ago

Good point. Thanks for the feedback!

mortenlerudjordet commented 1 week ago

IMO, not finding something (for supported param and type) should never be an error. Maybe an warning, but best is that the return is just a $null.

jikuja commented 1 week ago

I've seen similar issues on multiple Cmdlets. The process:

Cmdlet uses e.g. Get-AzAdUser to resolve UPN->object ID mapping, group name -> object ID or service principal name -> object id
Cmdlet returns nothing => Error Message without proper reason: in this case passing null as object id
Cmdlet returns more that one entry => Error Message without proper reason trying to pass array instead of single object id

exatly same issue is present also on Add-AzGroupMember: https://github.com/Azure/azure-powershell/blob/main/src/Resources/MSGraph.Autorest/custom/Add-AzADGroupMember.ps1#L119

aolszowka commented 3 days ago

IMO, not finding something (for supported param and type) should never be an error. Maybe an warning, but best is that the return is just a $null.

+1 on this comment; you really shouldn't be throwing exceptions/errors in non-exceptional circumstances...

hpatel292-seneca commented 3 days ago

Hi @isra-fel, Can I work on this issue?

Azure / azure-powershell