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.

Sign up for GitHub

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

Jump to bottom

fix attribute mapping for ManagedIdentityToken #40268

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix attribute mapping for ManagedIdentityToken #40268

wants to merge 1 commit into from

Conversation

jimdigriz
Copy link

@jimdigriz jimdigriz commented Mar 28, 2025 •
edited
Loading

As per the documentation the response to listTokens is formatted as:

{"value":[{"authorizationAudience":"https://management.azure.com/","resourceId":"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Solutions/applications/{applicationName}","access_token":"...","expires_in":"86399","expires_on":"1743242070","not_before":"1743155371","token_type":"Bearer"}]}

The current mapping expects all the attributes to be in camel case, when the token parts are actually in snake; in particular 'access_token' and 'token_type'.

This commit fixes the mapping so that <ManagedIdentityToken>.access_token no longer always returns None.

@github-actions GitHub Actions
Copy link

Thank you for your contribution @jimdigriz! We will review the pull request and get back to you soon.

@github-actions github-actions bot added Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Mar 28, 2025
@jimdigriz
fix attribute mapping for ManagedIdentityToken
cf841d8 
As per the documentation[1] the response to listTokens is formatted as:
----
{"value":[{"authorizationAudience":"https://management.azure.com/","resourceId":"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Solutions/applications/{applicationName}","access_token":"...","expires_in":"86399","expires_on":"1743242070","not_before":"1743155371","token_type":"Bearer"}]}
----

The current mapping expects all the attributes to be in camel case, when the token parts are actually in snake; in particular 'access_token' and 'token_type'.

This commit fixes the mapping so that <ManagedIdentityToken>.access_token no longer always returns None.

[1] https://learn.microsoft.com/en-us/azure/azure-resource-manager/managed-applications/publish-managed-identity#accessing-the-managed-identity-token
@jimdigriz jimdigriz force-pushed the managedapplication_list_tokens_fix branch from 04ace1e to cf841d8 Compare March 28, 2025 16:33
@annatisch
Copy link
Member

Thanks for this patch @jimdigriz!
Unfortunately this SDK is generated from a REST API spec, and any manual edits to the code will be undone next time the code is generated - so we need to fix the original specification.
@msyyc - could you open an issue in the spec repo to have this addressed?

@msyyc
Copy link
Member

msyyc commented Apr 3, 2025 

ce":"https://management.azure.com/","resourceId":"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Solutions/applications/{applicationName}","access_token":"...","expires_in":"86399","expires_on":"1743242070","not_before":"1743155371","token_type":"Bearer"}]}

The current mapping expects all the attributes to be in camel cas

Hi @jimdigriz this SDK model is generated from https://github.com/Azure/azure-rest-api-specs/blob/166c3a2ace3d2511cbdeeada95826cd67a45895e/specification/solutions/resource-manager/Microsoft.Solutions/preview/2023-12-01-preview/managedapplications.json#L2849-L2881. Please file an issue in https://github.com/Azure/azure-rest-api-specs/issues. After service team fix the swagger, SDK team will release a new SDK verison to fix it.

@jimdigriz jimdigriz mentioned this pull request Apr 3, 2025
Open
@jimdigriz
Copy link
Author

jimdigriz commented Apr 3, 2025 •
edited
Loading

After service team fix the swagger, SDK team will release a new SDK verison to fix it.

They have 1800+ issues, guess I should not be holding my breath? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChenxiJiang333 ChenxiJiang333 Awaiting requested review from ChenxiJiang333 ChenxiJiang333 is a code owner

@msyyc msyyc Awaiting requested review from msyyc msyyc is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jimdigriz @annatisch @msyyc