Dynamic authorization policy coverage updates #35476
Conversation
guardrex
changed the title
|
|
||
| ASP.NET Core 8 introduces the <xref:Microsoft.AspNetCore.Authorization.IAuthorizationRequirementData> interface. The `IAuthorizationRequirementData` interface allows the attribute definition to specify the requirements associated with the authorization policy. Using `IAuthorizationRequirementData`, the preceding custom authorization policy code can be written with fewer lines of code. The updated `Program.cs` file: | ||
| .NET 8 introduces the <xref:Microsoft.AspNetCore.Authorization.IAuthorizationRequirementData> interface. The `IAuthorizationRequirementData` interface allows the attribute definition to specify the requirements associated with the authorization policy. Using `IAuthorizationRequirementData`, the preceding custom authorization policy code can be written with fewer lines of code. The updated `Program.cs` file: |
There was a problem hiding this comment.
| .NET 8 introduces the <xref:Microsoft.AspNetCore.Authorization.IAuthorizationRequirementData> interface. The `IAuthorizationRequirementData` interface allows the attribute definition to specify the requirements associated with the authorization policy. Using `IAuthorizationRequirementData`, the preceding custom authorization policy code can be written with fewer lines of code. The updated `Program.cs` file: | |
| ASP.NET Core 8 introduced the <xref:Microsoft.AspNetCore.Authorization.IAuthorizationRequirementData> interface. The `IAuthorizationRequirementData` interface allows the attribute definition to specify the requirements associated with the authorization policy. Using `IAuthorizationRequirementData`, the preceding custom authorization policy code can be written with fewer lines of code. The updated `Program.cs` file: |
Minor item, but I think this should stay "ASP.NET Core" shouldn't it since it was Microsoft.AspNetCore specific.
There was a problem hiding this comment.
The "Core" language was retained to keep it distinct from the .NET Framework; although, little in this doc set would be confusing at this point on that aspect. These references are just referring to the release, and ASP.NET Core never had a different release cycle than .NET. Therefore, it's less wordy to just say ".NET X" when discussing a release. Otherwise, the reader has to read past all of these extra words.
This is one that we need a ruling on. @danroth27, when we're just referring to a release, can we just write ".NET X introduced ...," or do you want readers to keep reading "ASP.NET Core X introduced ..."? If you prefer the latter, I'll need to sweep the Blazor node. I'm always keen on using less words where I can to speed readers up, and Blazor docs rely heavily on just ".NET X" language.
There was a problem hiding this comment.
Its a minor thing and does not need to hold up your PR. Determining what to do going forward is good though. My thinking was that if a change is specific to just ASP.NET Core, as opposed to wider .NET scope then make that clear so it does not give the impression the new feature is used outside of ASP.NET Core for other app types.
There was a problem hiding this comment.
It's just referring to the release. The main distinction was for ASP.NET Core versus .NET Framework; but now that the versions have rolled well beyond 4/4.x, that's probably not confusing any longer.
Let's sit on this for the day to hear what Dan prefers. I'll need to sweep the Blazor node tho if we're going to "ASP.NET Core"-all-the-things 😄 . It seems wordy ... distracting ... to me these days to keep all of those extra letters 😆. I guess I'm a lazy 🦖 reader!
If we don't hear back by EOD, I'll merge this ... and then issue a patch PR later if Dan wants the full, explicit name.
|
Approved. I'll leave that minor item up to you. |
|
@danroth27 ... I'm going to go ahead and merge this to get rid of a docs build error on other PRs. If I hear back from you that we need to always use "ASP.NET Core X" when referring to a release, instead of just ".NET X," I'll issue a patch PR for this and open an issue to sweep the Blazor node for potential changes. |
Fixes #35467
Updates are ready for a look 👀. The sample app has already been updated to support these changes.
It turns out that the original PR submitted by the community member was closer to the best direction to take because the PU sample actually uses the pre-.NET 8 approach (i.e., it doesn't adopt IAuthorizationRequirementData (.NET 8 Release Notes)). Therefore, it was best to fix up our sample app, update it to .NET 9, and leave it cross-linked into the article.
If I don't mind saying so myself 🙈😆, I especially like the fleshed-out Demonstration section. We had just a couple of quick command shell lines before, but now we'll have a complete description of how to test with the sample app. This is an important set of auth concepts to master, so I felt a clear, step-by-step demo makes sense for this. Because I'm also showing the output, they don't really need to run the sample if they don't want to.
BTW ... I'm performing a small test here of
diffcode block spacing. My impression is just that we don't add an extra space after the-/+signs on the code change lines. If this doesn't work out after merging to live, I'll submit a follow-up patch PR to add one space on those lines after the-/+characters.Cross-refs:
Internal previews