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

Add .WithKnownB2CAuthorityHost #911

Closed
jmprieur opened this issue Feb 25, 2019 · 15 comments
Closed

Add .WithKnownB2CAuthorityHost #911

jmprieur opened this issue Feb 25, 2019 · 15 comments
Assignees
Labels

Comments

@jmprieur
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Enable B2C developers to tell MSAL about their known authority hosts, and the corresponding OAuth2.0 metadata

Describe the solution you'd like
Add the possibility of adding to the ApplicationBaseBuilder the following method:
.WithKnownB2CAuthorityHost(string authorityHost, struct describing the metadata)

Describe alternatives you've considered
see App developer is protected by authority validation when using *.b2clogin.com

@bgavrilMS
Copy link
Member

It seems that the current strategy is the one that is not recommended. I propose we take this bug before GA.

@bgavrilMS bgavrilMS added this to the 3.0.3 milestone Apr 4, 2019
@bgavrilMS
Copy link
Member

bgavrilMS commented Apr 4, 2019

Alternative proposal:

  • For B2C, the authority must be specified at the PCA level (but we won't enforce this here)
  • AcquireToken* calls cannot specify an Authority that has a different host than the one in PCA

In fact we can generalize this for all Authority types - just bear in mind that for AAD authorities have aliases. I think it's ok to force the user to specify the same authority in both PCA and AcquireToken*, but if you think differently, we can always perform InstanceDiscovery and take aliases into account.

@jmprieur @jennyf19 - would this work?

@jmprieur jmprieur modified the milestones: 3.0.3, 3.0.4 Apr 4, 2019
@jennyf19
Copy link
Collaborator

jennyf19 commented Apr 5, 2019

@bgavrilMS I don't think this will work for B2C because you can do different things in each acquire token call, without having to create a new client application.
For example, you can do an AT call for signIn/signUp policy and then do edit profile, which is a different authority and different token. So devs will have to make new client applications each time they want to target a new policy.

Another possibility would be something like this:

application = PublicClientApplicationBuilder.Create(ClientID)
               .WithB2CAuthorityHostInfo(authorityHost, tenantId)
               .Build();

so they give the authorityHost like: fabrikamb2c.b2clogin.com and the tenantId fabrikamb2c.onmicrosoft.com and we construct the full authority (so no more worrying about tfp)

Then in the AT builders, they would just pass in the policy:

app.AcquireTokenXXX(scopes, mandatory-parameters)
   .WithB2CPolicy(si_su_policy).
   .ExecuteAsync(cancellationToken);

We'd take the authority from the client application builder and append the policy.
thoughts?

@bgavrilMS
Copy link
Member

bgavrilMS commented Apr 5, 2019

I like the high level API that you propose, we can even provide a bunch of constants for .WithB2CPolicy, or an enum, or both.

So in the B2C case, if the developers still use:
app.AcquireTokenXXX.WithAuthority then we will throw.

On my proposal, developers can use any policy they like, because the host part of the URI doesn't change.

@jennyf19
Copy link
Collaborator

jennyf19 commented Apr 5, 2019

@bgavrilMS I think, currently, if a b2c developer uses .WithAuthority() and passes in a b2c authority, we do not throw, but instead try to do instance discovery and fail because we don't know it's a b2c authority in that case.
Also, for the policies, they can be anything the developer chooses, so what do you mean by an enum or constants?

@jmprieur jmprieur added the B2C label Apr 5, 2019
@jmprieur
Copy link
Contributor Author

jmprieur commented Apr 5, 2019

I like you proposal a lot @jennyf19, and I've discussed something similar with @valnav who also liked it. The thing is that by explicating a .WithB2CPolicy, we can also take care of other aspects like get the account for the policy, and the prompt=none for some policies. this requires a bit more spec, but I'd really like to go there.
Note that the .WithB2CAuthority at app building is a breaking change => we'd want to take it now, I guess. I would not take the .WithB2CPolicy now, though as this is additive (as much as I would love to have it, I prefer we GA first and anyway it needs to go through API review / SDK governance)

@jennyf19
Copy link
Collaborator

jennyf19 commented Apr 5, 2019

@parakhj - thoughts on the above proposal?

@parakhj
Copy link

parakhj commented Apr 5, 2019

Can we call it IEF policy? I know it's silly, but B2C runs on top of "IEF (Identity Experience Framework)", so really it's an IEF policy. Now we can argue that most developers don't know what IEF is, which is a valid argument, but IEF policy is technically accurate than "B2C policy"

@jennyf19
Copy link
Collaborator

jennyf19 commented Apr 6, 2019

@parakhj thanks for the insight. much appreciated.

@jennyf19
Copy link
Collaborator

jennyf19 commented Apr 8, 2019

@jmprieur here's a protoype, needs some work still and a couple u tests are failing, but have the Xamarin UWP app hooked up if you want to try it out.

@jmprieur
Copy link
Contributor Author

jmprieur commented Apr 8, 2019

@parakhj do we expect IEF to be surfaced to customers in the future? I'm worried that B2C developers (who know they want to use B2C) don't discover easily that they need to use WithIEFPolicy? or would they?

@parakhj
Copy link

parakhj commented Apr 8, 2019

@valnav what are your thoughts on this?

@jennyf19
Copy link
Collaborator

jennyf19 commented Apr 9, 2019

@markti thoughts on the above?

@valnav
Copy link

valnav commented Apr 9, 2019

@valnav what are your thoughts on this?

B2C is more close to the scenario they are building today. Also, using IEF may confuse the developer between User Flows and Custom Policies.

However, B2C as you say may not work as well - cant we stick with what we have in Microsoft Graph which is TrustFrameworkPolicy

@jmprieur
Copy link
Contributor Author

Closing as we've decided not to particularize B2C in public API

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

No branches or pull requests

6 participants