RequiredPolicy reborn and less demanding as FallbackPolicy

[HaoK]

Revert changes to required policy and auth z middleware, switch required policy behavior to be a fallback policy instead which is only used when Combine is called with no IAuthorizeData.

Note this has no impact on IAuthorizationService, callers would be able to take advantage of this feature by calling the appropriate Combine (which is aware of Fallback policy), but there's nothing in the service which will use this policy automatically.

[HaoK]

Initial stab at what we discussed today with @rynowak @Tratcher @blowdart

cc @pranavkm @JamesNK

[davidfowl]

I kinda want to make new methods ValueTask

[HaoK]

So if we really don't want to do a breaking change (and/or don't want to introduce a full on new concept), what about a slightly less flexbile option called UseDefaultPolicyAsFallback = true bool, which wouldn't require a new Policy, and the middleware would just use the default policy as the fallback which satisfies most of the scenarios we care about still right?

[rynowak]

So if we really don't want to do a breaking change (and/or don't want to introduce a full on new concept), what about a slightly less flexbile option

I think it's worth proposing that we make a breaking change since this adds a new concept that we want users to find.

If the intent is for this type to grow horizontally (adding new methods and properties) then it should be a class so you can do that, or we need to use default interface methods. Even if we're allowed to make a breaking change, it's still painful for the community and should be avoided where possible.

[HaoK]

Ok added the default interface method, so this is no longer a breaking change as a result right?

[HaoK]

So I as able to build locally with the new interface default method but the CI builds are not happy. Do I need to flip a flag somewhere for the CI?

[Tratcher]

@aspnet/build Are default interfaces available yet?

[dougbu]

@Tratcher as far as I can tell, the default interface feature didn't make it into C# 8 yet. No idea how your local tests are working @HaoK.

[HaoK]

I'm just running dotnet test on the AuthZ tests, maybe that builds against ref assemblies which aren't affected? dotnet build in the AuthZ project works too, no idea

[natemcmaster]

dotnet/csharplang#406 and https://github.com/dotnet/csharplang/blob/master/proposals/default-interface-methods.md say the feature is in-progress. We're using whichever version of the C# compiler is bundled in the .NET Core SDK (controlled by global.json). From the repo root, you can run this to find the compiler version: dotnet .dotnet/x64/sdk/3.0.100-preview4-011136/Roslyn/bincore/csc.dll /version. At the moment, that means we have Roslyn 3.100.19.20602 (0bafe122).

[HaoK]

So what should do I about this PR in the short term without default interface implementations, should we just take a breaking change now and add the default implementation when available? (File a tracking issue)

[rynowak]

So what should do I about this PR in the short term without default interface implementations, should we just take a breaking change now and add the default implementation when available? (File a tracking issue)

That seems like a good way to make progress

[HaoK]

Filed #10222 to track the default interface issue