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] Fix the weird DI anti-pattern: Resolving ITokenAcquisition but then cast it to ITokenAcquisitionInternal. #240

Closed
jmprieur opened this issue Jun 23, 2020 · 1 comment

Comments

@jmprieur
Copy link
Collaborator

jmprieur commented Jun 23, 2020

Which Version of Microsoft Identity Web are you using ?
Microsoft Identity Web 0.1.5-preview

Is this a new or existing app?

Repro

Look at:
WebAppCallsWebApiAuthenticationBuilderExtensions.cs#L129
WebAppCallsWebApiAuthenticationBuilderExtensions.cs#L163

Expected behavior

We'd want to write code requesting only the ITokenAcquisitionInternal

                options.Events.OnAuthorizationCodeReceived = async context =>
                {
                    var tokenAcquisition = context.HttpContext.RequestServices.GetRequiredService<ITokenAcquisitionInternal>();
                    await tokenAcquisition.AddAccountToCacheFromAuthorizationCodeAsync(context, options.Scope).ConfigureAwait(false);
                    await codeReceivedHandler(context).ConfigureAwait(false);
                };
                options.Events.OnRedirectToIdentityProviderForSignOut = async context =>
                {
                    // Remove the account from MSAL.NET token cache
                    var tokenAcquisition = context.HttpContext.RequestServices.GetRequiredService<ITokenAcquisitionInternal>();
                    await tokenAcquisition.RemoveAccountAsync(context).ConfigureAwait(false);
                    await signOutHandler(context).ConfigureAwait(false);
                };

How to achieve this?

** Possible design**

  1. Have ITokenAcquisitionInternal inherit from ITokenAcquisition
  2. In ServiceCollectionExtensions.cs update the AddTokenAcquisition method to register the ITokenAcquisition service and the ITokenAcquistionInternal service, but turning out to be the same instance.
             services.AddScoped<ITokenAcquisition, TokenAcquisition>();
             services.AddScoped<ITokenAcquisitionInternal>(s => (ITokenAcquisitionInternal)s.GetService<ITokenAcquisition>());

(and same for the singleton)

  1. Change WebAppCallsWebApiAuthenticationBuilderExtensions.cs and WebAppCallsWebApiAuthenticationBuilderExtensions.cs to use only ITokenAcquisitionInternal
@jennyf19
Copy link
Collaborator

Included in 0.2.0-preview release

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

No branches or pull requests

3 participants