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

[Feature Request] Add a new WithClientAssertion API that exposes the token endpoint #3352

Closed
bgavrilMS opened this issue May 26, 2022 · 7 comments · Fixed by #3376
Closed
Labels
Feature Request ICM This issue has a corresponding ICM, either for our team or another.
Milestone

Comments

@bgavrilMS
Copy link
Member

Applications that create their own client assertion typically do it as:

WithClientAssertion( "aud: ${input_authority}/v2.0/token, iss: ${client_id}")

However, MSAL may decide to change the input_authority, because ESTS wants the SDK to use the "preferred_network". For example, for the public cloud, if the app developer configures "sts.windows.net/12345", MSAL will use "login.microsoftonline.com/12345". And it looks like ESTS does not support sending client_assertion with aud: sts.windows.net/12345/ over login.microsoftonline.com/12345. These 2 must match.

Proposal:

WithClientAssertion(Func<RequestDetails, Task<string>>);

// where RequestDetails has following properties: 
// TokenEndpoint
// ClientId
// CancellationToken

CC @jmprieur

@bgavrilMS bgavrilMS added Feature Request ICM This issue has a corresponding ICM, either for our team or another. labels May 26, 2022
@jmprieur
Copy link
Contributor

Thanks @bgavrilMS : how was this discovered?

@bgavrilMS
Copy link
Member Author

@jmprieur - via ICM, it looks like ESTS errors out if the authority from the assertion (the aud claim) != token endpoint where we ask the token.

The service impacted is creating their own client assertion, I think they are trying to use managed identity.

@jmprieur
Copy link
Contributor

@bgavrilMS : I think that there is an app registration issue in this case. This is the certless scenario.
Would you mind sharing their contact info offline?

@svrooij
Copy link
Contributor

svrooij commented May 27, 2022

To create a client assertion in the Key Vault as described in #3355 we need to know the value for the aud claim and the value for the iss and sub claims (which are both the Client ID).

My guess is that the specific information needed is already available in the used method, though not sure on the exact details.

Can I some how contribute code to fix this issue and 3355?

@bgavrilMS
Copy link
Member Author

@svrooij - I am discussing with the server side folks to fix this on their end, as it should be possible to provide an aud claim with that has an alias of the environment (i.e. windows.sts.net is an alias of login.microsoftonline.com). This works when using a normal certificate-based assertion, but seems to not work with managed identity.

Until this discussion is finalized, you can use one of MSAL's extensiblity APIs for your scenario - OnBeforeTokenRequest - which allows you to add / remove body params from the token request. See this PR for an example.

@svrooij
Copy link
Contributor

svrooij commented May 30, 2022

There is already a Task<string> (cancellationToken) way to async create the assertion. Having that code provide the full aud (the correct hostname and the tenant id) and the client id would allow to do it in the confidential client builder.

Using the extension OnBeforeToken... Would mean that you have to do it everywhere where you're using the cofidential client. Instead of adding it to the builder.

@bgavrilMS
Copy link
Member Author

Yes, but OnBeforeTokenRequest is in MSAL today :). The better way to create an assertion needs to be added as a new feature.

svrooij added a commit to svrooij/microsoft-authentication-library-for-dotnet that referenced this issue Jun 8, 2022
WithClientAssertion did not have access to client id or token endpoint
Which can be very useful

Fixed AzureAD#3352
Related to AzureAD#3355
bgavrilMS pushed a commit to svrooij/microsoft-authentication-library-for-dotnet that referenced this issue Jun 17, 2022
WithClientAssertion did not have access to client id or token endpoint
Which can be very useful

Fixed AzureAD#3352
Related to AzureAD#3355
bgavrilMS pushed a commit that referenced this issue Jun 17, 2022
* Access Client ID and TokenEndpoint

WithClientAssertion did not have access to client id or token endpoint
Which can be very useful

Fixed #3352
Related to #3355

* Marking WithAssertion obsolete

* Removed sync version of delegate

* removed full qualification
bgavrilMS pushed a commit that referenced this issue Jun 17, 2022
* Access Client ID and TokenEndpoint

WithClientAssertion did not have access to client id or token endpoint
Which can be very useful

Fixed #3352
Related to #3355

* Marking WithAssertion obsolete

* Removed sync version of delegate

* removed full qualification
This was referenced Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request ICM This issue has a corresponding ICM, either for our team or another.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants