Skip to content

Conversation

@jiasli
Copy link
Member

@jiasli jiasli commented May 19, 2022

Related command
az ad

Description

Currently, all subclasses of CLIError are re-raised as GraphError. This is not correct and causes failure for AuthenticationError handling.

For example, we change microsoft_graph_resource_id to an invalid value like https://graph1.microsoft.com/, which is then used as a scope to get an access token:

microsoft_graph_resource_id='https://graph.microsoft.com/',

The error handling procedure failed:

 > az ad app show --id 0e33fcf1-8a78-4941-9479-75d29681f4dc
...
    result = self._send("GET", "/applications" + _filter_to_query(filter))
  File "d:\cli\azure-cli\src\azure-cli\azure\cli\command_modules\role\msgrpah\_graph_client.py", line 40, in _send
    raise GraphError(ex.response.json()['error']['message'], ex.response) from ex
AttributeError: 'AuthenticationError' object has no attribute 'response'

This PR makes send_raw_request raise a dedicated error HTTPError and it is then re-raised as GraphError. Since HTTPError is a sub-class of CLIError, it is backward compatible.

Now invalid scope will result in

> az ad app show --id 0e33fcf1-8a78-4941-9479-75d29681f4dc
This command or command group has been migrated to Microsoft Graph API. Please carefully review all breaking changes introduced during this migration: https://docs.microsoft.com/cli/azure/microsoft-graph-migration
AADSTS500011: The resource principal named https://graph1.microsoft.com/ was not found in the tenant named AzureSDKTeam. This can happen if the application has not been installed by the administrator of the tenant or consented to by any user in the tenant. You might have sent your authentication request to the wrong tenant.
Trace ID: 4afc0638-c613-4eba-8556-0031e7c8a500
Correlation ID: 17b08664-450c-41fa-a69b-09002c9d5c4d
Timestamp: 2022-05-19 06:52:07Z
To re-authenticate, please run:
az login --scope https://graph1.microsoft.com//.default

@ghost ghost requested a review from yonzhan May 19, 2022 07:36
@ghost ghost added the Auto-Assign Auto assign by bot label May 19, 2022
@ghost ghost assigned jiasli May 19, 2022
@ghost ghost added this to the May 2022 (2022-05-24) - For Build milestone May 19, 2022
@ghost ghost added the RBAC az role label May 19, 2022
@ghost ghost requested a review from wangzelin007 May 19, 2022 07:36
@ghost ghost added the Graph az ad label May 19, 2022
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative names can be RequestError or RawRequestError. I chose this name similar to requests.exceptions.HTTPError.

@yonzhan
Copy link
Collaborator

yonzhan commented May 19, 2022

Role

@jiasli jiasli changed the title {Role} Only raise GraphError when Graph request fails {Role} Raise GraphError only when Graph request fails May 19, 2022
@jiasli jiasli merged commit 3dbcafb into Azure:dev May 19, 2022
@jiasli jiasli deleted the graph-error branch May 19, 2022 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Auto-Assign Auto assign by bot Graph az ad RBAC az role

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants