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

[BUG] Access token parsing failed #23261

Closed
ritishgoyal opened this issue Aug 11, 2021 · 7 comments · Fixed by #23794
Closed

[BUG] Access token parsing failed #23261

ritishgoyal opened this issue Aug 11, 2021 · 7 comments · Fixed by #23794
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Attention Workflow: This issue is responsible by Azure service team.

Comments

@ritishgoyal
Copy link

ritishgoyal commented Aug 11, 2021

Describe the bug
When I try to get an access token for an external resource using Client certificate token provider by using AzureServiceTokenProvider.GetAccessTokenAsync(), I receive an error saying the access token couldn't be parsed.

Expected behavior
We should not try to parse the access token since it is not in the format expected.

Actual behavior (include Exception or Stack Trace)
Microsoft.Azure.Services.AppAuthentication.AzureServiceTokenProviderException: Parameters: Connection String: RunAs=App;AppId=e165870b-8f4f-48ad-951f-2752be329850;TenantId=cdc5836a-15c5-4db6-b079-fcadd2505dc2;CertificateSubjectName=CN=sp.m365insidexxxxx.gbl;CertificateStoreLocation=CurrentUser, Resource: api://04cbdc38-6aaf-4668-bc55-b12e857433, Authority: https://login.microsoftonline.com/cdc5836a-15c5-4db6-b079-fcadd2505dc2. Exception Message: Tried 1 certificate(s). Access token could not be acquired.
Exception for cert #1 with thumbprint 6E199A1F059912D67xxxxx: Access token is not in the expected format. Exception: There was an error deserializing the object of type Microsoft.Azure.Services.AppAuthentication.AccessToken. Encountered unexpected character 'w'.
at Microsoft.Azure.Services.AppAuthentication.AzureServiceTokenProvider.d__19.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.Azure.Services.AppAuthentication.AzureServiceTokenProvider.d__21.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()

To Reproduce
var azureServiceTokenProvider = new AzureServiceTokenProvider($"RunAs=App;AppId={"xxxxx"};TenantId={"zzzz"};CertificateSubjectName=CN={"xxxx"};CertificateStoreLocation=CurrentUser");
string token = await azureServiceTokenProvider.GetAccessTokenAsync("https:\resourcexx")

I was able to fix the issue by removing line 197: PrincipalUsed.TenantId = AccessToken.Parse(accessToken).TenantId; from ClientCertificateAccessTokenProvider.cs
So, the issue is that the returned access token is not in the format expected by the library but is still a valid token.

Environment:

  • Name and version of the Library package used: Microsoft.Azure.Services.AppAuthentication 1.6.1
  • Hosting platform or OS and .NET runtime version (dotnet --info output for .NET Core projects): .NET Framework 4.6.3
@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Aug 11, 2021
@jsquire jsquire added AppAuthentication Client This issue points to a problem in the data-plane of the library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Service Attention Workflow: This issue is responsible by Azure service team. labels Aug 11, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Aug 11, 2021
@jsquire
Copy link
Member

jsquire commented Aug 11, 2021

//cc: @nonik0, @anaismiller

@jsquire
Copy link
Member

jsquire commented Aug 11, 2021

Thank you for your feedback. Tagging and routing to the team members best able to assist.

@anaismiller
Copy link
Contributor

@ritishgoyal What are you trying to use the certificate for? Is is a supported scenario?

@ritishgoyal
Copy link
Author

ritishgoyal commented Aug 18, 2021

@anaismiller I am trying to get a token for an external azure resource using s2s App ID based auth with an App ID and certificate.
It should be a valid scenario since the token fetched succeeds in authorization for my s2s calls, only if I remove the parsing logic which is throwing an error. I am thinking of raising a pull request for making the token parsing to get tenantID optional.

@ritishgoyal
Copy link
Author

Can you provide an update on this?

@nonik0
Copy link
Contributor

nonik0 commented Aug 27, 2021

@ritishgoyal Thanks for pointing this out and sorry for the delay. While I am not sure about the specifics of your scenario, it does like like you're getting a non-standard token from AAD (are your access tokens being encrypted?). We do have some non-ideal code that parses the token from AAD to get tenant ID info, but this is not strictly necessary. It's generally not good practice either to "look" at access tokens in code, and should rather be treated opaquely.

I can discuss options with the team but so far your suggestion of making the tenantID parsing logic optional (i.e. eat parsing exception) is the best option so far IMO.

@ritishgoyal
Copy link
Author

I'm not sure why the access token is non-standard. I'm getting it from the AAD production endpoint.
Yes, I think eating the parsing exception is the right call. Can you follow up on this with your team and get a fix in soon.
I'm kind of blocked on this and currently using a custom compilation of the library to get around this. Thanks!

@anaismiller anaismiller linked a pull request Sep 17, 2021 that will close this issue
11 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Attention Workflow: This issue is responsible by Azure service team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@jsquire @anaismiller @nonik0 @ritishgoyal and others