Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jikuja opened this issue Oct 8, 2024 · 5 comments
Labels
Azure PS Team bug This issue requires a change to an existing behavior in the product in order to be resolved. customer-reported good first issue Issues suited for folks who want to help for the first time. Graph.Microsoft Tracking We will track status and follow internally

Comments

@jikuja
Copy link

jikuja commented Oct 8, 2024

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
@jikuja jikuja added bug This issue requires a change to an existing behavior in the product in order to be resolved. needs-triage This is a new issue that needs to be triaged to the appropriate team. labels Oct 8, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added customer-reported needs-triage This is a new issue that needs to be triaged to the appropriate team. and removed needs-triage This is a new issue that needs to be triaged to the appropriate team. labels Oct 8, 2024
@isra-fel
Copy link
Member

Good point. Thanks for the feedback!

@isra-fel isra-fel added Azure PS Team Graph.Microsoft good first issue Issues suited for folks who want to help for the first time. Tracking We will track status and follow internally and removed needs-triage This is a new issue that needs to be triaged to the appropriate team. labels Oct 10, 2024
@mortenlerudjordet
Copy link

mortenlerudjordet commented Oct 10, 2024

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
Copy link
Author

jikuja commented Oct 11, 2024

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
Copy link

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure PS Team bug This issue requires a change to an existing behavior in the product in order to be resolved. customer-reported good first issue Issues suited for folks who want to help for the first time. Graph.Microsoft Tracking We will track status and follow internally
Projects
None yet
Development

No branches or pull requests

5 participants