Skip to content

[Bug] SingletonTokenAcquisition can be removed #249

Closed
Closed
[Bug] SingletonTokenAcquisition can be removed#249
Assignees
jmprieur
Labels
.NET Core 5P2code cleanupfixedreported-by-aspnetcore
Milestone
[1] .NET 5 Readiness - 0.2.0-preview

Description

@jmprieur
Jean-Marc Prieur (jmprieur)
opened on Jun 24, 2020

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

AddTokenAcquisition isTokenAcquisitionSingleton parameter (never used)

microsoft-identity-web/src/Microsoft.Identity.Web/ServiceCollectionExtensions.cs

Lines 32 to 34 in 0b628e2

public static IServiceCollection AddTokenAcquisition(
this IServiceCollection services,
bool isTokenAcquisitionSingleton = false)
is a request from the Microsoft Graph SDK which want a singleton (whereas in general that’s not such a good idea)

See https://github.com/AzureAD/microsoft-identity-web/blob/master/tests/WebAppCallsMicrosoftGraph/appsettings.json#L9

There is an alternative which is that the GraphService registers the ITokenAcquisition as a singleton (the last which does the registration)

Decision:

  • We keep AddTokenAcquisition as is
  • We remove the SingletonTokenAcquisition property from the Microsoft.Identity.Options
  • AddTokenAcquision is called with the default value (false) from the extension methods
  • Remove the tests that were testing this.

Activity

  1. Jean-Marc Prieur (jmprieur)
    added
    P2
    reported-by-aspnetcore
    code cleanup
    on Jun 24, 2020
  2. Jean-Marc Prieur (jmprieur)
    self-assigned this
    on Jun 24, 2020
  3. jennyf19
    modified the milestone: .NET 5 Readiness - 0.2.0-preview on Jun 24, 2020
  4. Jean-Marc Prieur (jmprieur)
    added
    .NET Core 5
    on Jun 24, 2020
  5. Jean-Marc Prieur (jmprieur)
    added this to the [1] .NET 5 Readiness - 0.2.0-preview milestone on Jun 26, 2020
  6. jennyf19
    added
    In progress
    on Jun 29, 2020
  7. Jean-Marc Prieur (jmprieur)
    added
    fixed
    and removed
    In progress
    on Jun 30, 2020

  8. jennyf19 commented on Jul 13, 2020

    @jennyf19
    jennyf19
    on Jul 13, 2020
    Collaborator

    Included in 0.2.0-preview release

  9. jennyf19
    closed this as completedon Jul 13, 2020

  10. sviatoslav-vilkovych commented on Sep 11, 2020

    @sviatoslav-vilkovych
    sviatoslav vilkovych (sviatoslav-vilkovych)
    on Sep 11, 2020

    So, there is no more posibility to have TokenAcquisition registered as a Singleton service, right?

  11. pmaytak commented on Sep 11, 2020

    @pmaytak
    Peter (pmaytak)
    on Sep 11, 2020
    Collaborator

    You still can. Call services.AddTokenAcquisition(true) at the end of ConfigureServices method. (The related PRs just removed SingletonTokenAcquisition property from MicrosoftIdentityOptions.)

    microsoft-identity-web/src/Microsoft.Identity.Web/ServiceCollectionExtensions.cs

    Lines 33 to 35 in 7df3679

    public static IServiceCollection AddTokenAcquisition(
    this IServiceCollection services,
    bool isTokenAcquisitionSingleton = false)

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

Metadata

Metadata

Assignees

Labels

.NET Core 5P2code cleanupfixedreported-by-aspnetcore

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions