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] instance aware support #2524

Closed
henrik-me opened this issue Apr 1, 2021 · 18 comments · Fixed by #3141
Closed

[Feature Request] instance aware support #2524

henrik-me opened this issue Apr 1, 2021 · 18 comments · Fixed by #3141
Assignees
Milestone

Comments

@henrik-me
Copy link
Contributor

henrik-me commented Apr 1, 2021

Feature 824881: [MSAL.NET] Sovereign cloud aware is supported by MSAL

If a client indicates that it's "instance_aware" then EVO will authenticated against the cloud associated with the account attempting to authenticate and will instruct the client library (MSAL) where to redeem the resulting authorization code for a token.

https://identitydivision.visualstudio.com/DevEx/_git/AuthLibrariesApiReview/pullrequest/1450?_a=files&path=%2F%5BXplat%5D%20Instance%20Aware%2Finstance_aware_flows.md

Auth Client spec: https://identitydivision.visualstudio.com/DevEx/_git/AuthLibrariesApiReview?path=%2F%5BXplat%5D%20Instance%20Aware%2Finstance_aware_flows.md&_a=contents&version=GBdev

.js implementation details:
MSAL.js v2: https://github.com/AzureAD/microsoft-authentication-library-for-js/pull/1804/files
Docs: https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-browser/docs/instance-aware.md

@pmaytak
Copy link
Contributor

pmaytak commented Apr 12, 2021

  • Interactive:
    • In InteractiveRequest.GetTokenResponseAsync after fetching auth code, if AuthenticationResult.CloudInstanceHost was set, update authority in AuthenticationRequestParameters and call ResolveAuthorityEndpointsAsync before calling the token endpoint.
    • Test: Request with instance_aware query. Verify token endpoint has updated host and token response is cached with correct environment.
  • Integrated Windows Auth:
    • In IntegratedWindowsAuthRequest.FetchAssertionFromWsTrustAsync, if the response is Unknown and contains cloud_instance_host_name; then update authority in AuthenticationRequestParameters with new host; create and send a new request to the updated authority; then call ResolveAuthorityEndpointsAsync before calling the token endpoint.
    • Test: Same as for interactive request.
  • Username/Password:
    • Same changes as for IWA, but in UsernamePasswordRequest.
    • Test: Same as for interactive request.
  • Create wiki page

@jennyf19
Copy link
Collaborator

Makes sense for PCA, but what about confidential client? The work has been done on android/ios, but they only support PCA, as noted in the above list, all PCA related concepts. I thought our initial concern was for impact on CCA...?

@bgavrilMS
Copy link
Member

@jennyf19 @jmprieur - can we configure ASP.NET Core to inject a query param for when it gets the auth code? This is how you enable instance_aware. What about ASP.NET Classic?

@pmaytak - WAM needs to be support this as well. I think it should, but we need to test.

@jennyf19
Copy link
Collaborator

@bgavrilMS we can add parameters to the auth code request, but i'm not familiar w/instance aware, so would have to understand it better. for ASP.NET Classic, people would mainly be using MSAL.NET and ID Web only for the cache serialization and cert loading.

@pmaytak
Copy link
Contributor

pmaytak commented Apr 13, 2021

@bgavrilMS I think something like the query param can be added in OnRedirectToIdentityProvider and then the response context in OnAuthorizationCodeReceived will have the cloud host. The host can be passed into CCA.AcquireTokenByAuthorizationCode.

@jennyf19
Copy link
Collaborator

@pmaytak that's what i meant by "we can add parameters to the auth code request", but after reading about instance_aware, I have opened a separate work item in Id web, which @jmprieur can prioritize against the other items there. The work in id web is not related to, nor dependent on, the work to be done in MSAL.NET.

@jennyf19
Copy link
Collaborator

Makes sense for PCA, but what about confidential client?

Additional MSAL.NET requests to consider are:

  • device code (PCA)
  • WAM (PCA)
  • OBO (CCA)
  • auth code (CCA)
  • byRefreshToken (CCA)

@pmaytak
Copy link
Contributor

pmaytak commented Apr 13, 2021

  • Device code
    • I don't think this applies as instance_aware is only for authorize endpoint.
  • OBO
    • I think there are no changes in MSAL. The user would request the token for the correct authority beforehand.
  • By auth code
    • In ConfidentialAuthCodeRequest.ExecuteAsync MSAL only calls the token endpoint using the authority from AuthenticationRequestParameters. So the client app when calling AcquireTokenByAuthorizationCode would also use WithAuthority with the updated authority of the cloud instance.
  • By refresh token
    • Same as by auth code.

@jmprieur
Copy link
Contributor

jmprieur commented Apr 13, 2021

@pmaytak:
My impression is that this is not so simple for confidential client applications:

  • for OBO, if the web API is a multi-tenant application, configured with a public cloud authority, (https://login.microsoftonline.com/common or https://login.microsoftonline.com/organizations), which authority should be used in MSAL.NET before doing an OnBehalfOf call?
  • for Auth code for confidential client application, if the app uses .WithAuthority with the cloud instance authority (to redeem the code), then what will happen when the web app acquires a token for an API using AcquireTokenSilent with a different set of scopes ? My understanding is that MSAL.NET would use the configured authority (probably public cloud), and this won't work? On the other hand, if the web app instantiates a confidential client application per request, with an authority being set to the cloud instance authority, everything works fine? Also, when the user navigates between pages in the web app, the identity of the user is maintained in the session cookie. Is the cloud instance part of the user claims ? or otherwise some information in this cookie?
  • What do you mean, by refresh token? is it acquire token silent? or AcquireTokenByRefreshToken ?

@bgavrilMS
Copy link
Member

bgavrilMS commented Apr 13, 2021

Reading the spec, it looks like this only works when contacting the /authorize endpoint or the /ws-trust endpoint (for IWA and some usages of ROPC). I.e. the response from these endpoints will tell us the sovereign authority .

So when you contact the /token endpoint, we must target the correct authority.

For AcquireTokenSilent the spec says there is nothing to do, as it is expected that the developer uses the IAccount and env of the interactive login, smth like

AuthenticationResult result = AcquireTokenInteractive(PublicCloud); // user logs into German Cloud

// later
AcquireTokenSilent(account: result.Account, authority: result.Authority); 

It is important to use the correct authority for silent flows.

To me, the big question is how will the developer code look like? You set a flag to true, but then what?

For Public Client

  1. Should we modify GetAccounts API to return accounts irrespective of their enviroment (today, we filter by app env)
  2. Should we allow AcquireTokenSilent(account) to use the account.Env instead of the app env?

For Confidential Client

  1. Is there a claim in the id token that will allow M.I.W. to target the correct authority for web api?
  2. Is this claim also in the AT so that we can use it in OBO?

For Public Client, we can read the MSAL.Android code and have a chat with someone who knows about this scenario.
For Confidential Client, we need to experiment.

@jmprieur
Copy link
Contributor

After meeting with the service, it turns out that:

  • instance aware is not yet supported by the service for confidential client applications
  • all the brokers already support it, so it's de-facto supported on iOS, Android, and Windows through WAM

Conclusion: we don't need to do the work

@bgavrilMS
Copy link
Member

bgavrilMS commented Apr 20, 2021

Instance Aware is an opt-in feature, so even if we choose to support it only via the brokers we need to enable it. As such we need to understand:

  • how to enable this feature on each broker (probably via WithExtraQueryParams)
  • do we need to create a new API for support, or will .WithExtraQueryParams be sufficient (probably not)
  • Testing on Android, iOS and Windows

I'm pretty sure all that needs doing is to pass in "instance_aware=true" as extra query param. We should be sending the extra query params to all brokerks, but this needs testing. For example, for WAM

@bgavrilMS bgavrilMS removed the wontfix label Apr 20, 2021
@bgavrilMS
Copy link
Member

I'm also unsure if we can say "supported only via brokers" since this creates a bad app developer experience. Our broker strategy is to fallback to browsers in case the broker is not available. In this case, the user will get an obscure error, as they will get an auth_code from German cloud but they try to redeem it against WW.

@pmaytak pmaytak removed their assignment Apr 20, 2021
@bgavrilMS bgavrilMS added this to the 4.41.0 milestone Jan 25, 2022
@bgavrilMS bgavrilMS removed the wontfix label Jan 25, 2022
@bgavrilMS
Copy link
Member

bgavrilMS commented Jan 25, 2022

Let's get this going with priority for CCA.
We can delay mobile and need more info for PCA when it comes to brokers (how does WAM, Authenticator etc. handle this).

@bgavrilMS bgavrilMS reopened this Jan 31, 2022
@neha-bhargava neha-bhargava self-assigned this Jan 31, 2022
@neha-bhargava
Copy link
Contributor

neha-bhargava commented Jan 31, 2022

@bgavrilMS The comments above mentions "instance aware is not yet supported by the service for confidential client applications". Did something change?

@neha-bhargava
Copy link
Contributor

Discussed offline with @bgavrilMS. Currently priority is to support instance_aware for AcquireTokenInteractive / AcquireTokenSilent on PCA using browser. Next step would be to get it work when broker is used. It is a consistency item with Android and iOS SDKs

@bgavrilMS
Copy link
Member

@neha-bhargava - can you pls create a separate work item for brokers to keep track?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants