[Bug] Caching by MethodInfo will return same cache for different distributed cache configurations

[rymeskar]

Which version of Microsoft Identity Web are you using?
Latest master

Where is the issue?

• Web app
  • Sign-in users
  • Sign-in users and call web APIs
• Web API
  • Protected web APIs (validating tokens)
  • Protected web APIs (validating scopes)
  • Protected web APIs call downstream web APIs
• Token cache serialization
  • In-memory caches
  • Session caches
  • Distributed caches
• Other (please describe)

In the helper method for applications that would like to use Id.Web and MSAL caching and do not use DI, there is a cache in order to remove redundant initialization.

This cache however has a bug of only caching by MethodInfo. Relying on equality or difference of the produced MethodInfo is problematic. For example, I showcase that same pattern as is used for Distributed Cache registration results in not adding newly configured caches. There will always be just key for the distributed cache. Here's the full repro.

[jennyf19]

Updating the thread here:

so this test is the issue.

It's supposed to check that we use the same cca instance, but when checking the Distributed cache in the last line # 45, instead of the token source being cache, it's identity provider. This was happening with the Delegate & object suggestions from you repo and got the same result.

For this #1390

branch to use

cc: @rymeskar

[jmprieur]

Closing, as I believe, we concluded this was a non-repro

[rymeskar]

I think this is still a bug @jmprieur . I created a more relevant repro.

      [Fact]
      public void BugRepro()
      {
          var cca1 = ConfidentialClientApplicationBuilder
                         .Create(TestConstants.ClientId)
                         .WithAuthority(TestConstants.AuthorityCommonTenant)
                         .WithClientSecret(TestConstants.ClientSecret)
                         .Build();
          var cca2 = ConfidentialClientApplicationBuilder
                         .Create(TestConstants.ClientId)
                         .WithAuthority(TestConstants.AuthorityCommonTenant)
                         .WithClientSecret(TestConstants.ClientSecret)
                         .Build();
          var cca3 = ConfidentialClientApplicationBuilder
                         .Create(TestConstants.ClientId)
                         .WithAuthority(TestConstants.AuthorityCommonTenant)
                         .WithClientSecret(TestConstants.ClientSecret)
                         .Build();
          var cca4 = ConfidentialClientApplicationBuilder
                         .Create(TestConstants.ClientId)
                         .WithAuthority(TestConstants.AuthorityCommonTenant)
                         .WithClientSecret(TestConstants.ClientSecret)
                         .Build();
          var cca5 = ConfidentialClientApplicationBuilder
                         .Create(TestConstants.ClientId)
                         .WithAuthority(TestConstants.AuthorityCommonTenant)
                         .WithClientSecret(TestConstants.ClientSecret)
                         .Build();
          var cca6 = ConfidentialClientApplicationBuilder
                         .Create(TestConstants.ClientId)
                         .WithAuthority(TestConstants.AuthorityCommonTenant)
                         .WithClientSecret(TestConstants.ClientSecret)
                         .Build();
          var cca7 = ConfidentialClientApplicationBuilder
                         .Create(TestConstants.ClientId)
                         .WithAuthority(TestConstants.AuthorityCommonTenant)
                         .WithClientSecret(TestConstants.ClientSecret)
                         .Build();
          var cca8 = ConfidentialClientApplicationBuilder
              .Create(TestConstants.ClientId)
              .WithAuthority(TestConstants.AuthorityCommonTenant)
              .WithClientSecret(TestConstants.ClientSecret)
              .Build();

          cca1.AddDistributedTokenCache(services =>
          {
              count++;
              services.AddDistributedMemoryCache();
          });
          Assert.Equal(1, count); // correct increase.

          cca2.AddDistributedTokenCache(services =>
          {
              count++;
              services.AddDistributedMemoryCache();
          });
          Assert.Equal(1, count); // up for discussion whether this is correct or not because the action reference is actually different.

          cca3.AddDistributedTokenCache(AddCache);
          Assert.Equal(1, count); // this needs to be a count increase

          cca4.AddDistributedTokenCache(AddCache);
          Assert.Equal(1, count); // this should not be a count increase

          cca5.AddDistributedTokenCache(s =>
          {
              s.AddDistributedMemoryCache();
              s.AddMemoryCache();
              s.AddLogging(l => l.AddEventLog());
              count++;
          });
          Assert.Equal(1, count); // this needs to be a count increase

          cca6.AddInMemoryTokenCache(AddCache);
          Assert.Equal(2, count); // this is correct count increase

          cca7.AddInMemoryTokenCache(s =>
          {
              s.AddDistributedMemoryCache();
              s.AddMemoryCache();
              s.AddLogging(l => l.AddEventLog());
              count++;
          });
          Assert.Equal(2, count); // this needs to be a count increase

          cca8.AddInMemoryTokenCache();
          Assert.Equal(2, count); // this needs to be a count increase
      }

[bgavrilMS]

Reopening as per feedback from Karel.

[bgavrilMS]

@pmaytak - can you also take a look at this please?