Remove CAS code

[tdykstra]

First batch of snippet changes, contributes to #837

[danmoseley]

Seems reasonable to me. Obviously I want to be sure we aren't opening up any security hole for .NET Framework folks.

@GrabYourPitchforks

1. are there any circumstances in which removing this declarative CAS will cause a security issue today on .NET Framework? I know that our stub ones on .NET Core still throw in a few cases (rather than no-op) but as far as I can see that's not in the declarative case.
2. is it a problem that users consulting docs for older versions like 3.5 will not see these in the samples anymore?

[Youssef1313]

@tdykstra These needs to have the csproj file right?
I think it's okay if I started on that before it's merged, right? It seems that it won't cause conflicts.

[tdykstra]

I don't expect conflicts, but I think it best to wait until:

• @GrabYourPitchforks responds to the questions from @danmosemsft, and
• @Thraka says CI is set up in the API repo. Then the addition of project files will trigger a build on the projects.

[Thraka]

@tdykstra I think you meant to say that I said "WHEN CI is set up"

[tdykstra]

Yes, that's what I meant, "Wait until @Thraka says ..."

[tdykstra]

@GrabYourPitchforks Per the questions from @danmosemsft is it OK to proceed with the plan to remove CAS types from code examples, starting with this PR?

[GrabYourPitchforks]

Spoke offline with Tom. I only saw one incident that would have a behavioral change with potentially negative security consequence: a [PrincipalPermission] attribute was removed from one method. In Full Framework, those attributes still do what users expect, even when the application is running in full trust.

In general I think our recommendation should be that old samples remain unmodified. Aside from the above mentioned case, it's unlikely removing an attribute will introduce a security hole, but removing the attribute could cause an application to stop working if it happens to be deployed on a hoster that still uses partial trust enforcement.

For new samples, we shouldn't use CAS attributes anywhere. And if we come across existing samples that were intended to target .NET Core / netstandard, we should remove CAS attributes from those samples as we encounter them. But performing a system-wide sweep of existing code seems ill-advised if we don't have a good way of drawing this distinction. It risks introducing too much churn and could break people.

[tdykstra]

@GrabYourPitchforks Thank you for reviewing and commenting -- I'll close this PR.

[danmoseley]

Hmm. Realistically the samples we have today will be in our docs for many years, and if we don't remove the CAS attributes they will indefinitely remain front and center in large numbers of topics. We know folks copy and paste samples, so fresh code will pick up more of these attributes. Conversely if we take this PR, the worst case is we break some samples - not running code - but we can fix that if it's reported: we have a way forward.

It feels like the wrong tradeoff to leave this in. @stephentoub what is your take.

[stephentoub]

If we were writing these samples afresh today to run on .NET Framework, would we add these attributes? I'd remove any attributes for which the answer is "no".

Are any of the samples focused on code that might be in libraries, such that we expect developers might copy/paste that code into a netstandard library? Would we expect the attributes to be used in such cases (expecting that the netstandard lib might run against .NET Framework)? If no, I'd remove those, too.

[GrabYourPitchforks]

If we were writing these samples afresh today to run on .NET Framework, would we add these attributes? I'd remove any attributes for which the answer is "no".

No. It should be considered incorrect for any new code - regardless of whether it targets Full Framework or Core - to include CAS attributes. The ASP.NET team stopped adding these attributes to new code in early 2013. And other .NET teams followed soon after.

There might be some exceptions for people who have to maintain existing code bases, but those are definitely exceptional cases.

[danmoseley]

Given this, I'd prefer to go ahead and remove them (proceed with this PR) given we don't think it would reduce security for anyone (assuming @tdykstra tweaks the [PrincipalPermission] you spotted). The risk of breaking a few samples seems manageable but more importantly it allows us to give the right guidance to customers today - realistically we are not going to rewrite these samples en masse.

removing the attribute could cause an application to stop working if it happens to be deployed on a hoster that still uses partial trust enforcement.

Levi, this would only be the case for new code, right? Obviously, samples don't affect existing working apps. So it would be in a context where someone is modifying and re-testing their app.

[GrabYourPitchforks]

Levi, this would only be the case for new code, right? Obviously, samples don't affect existing working apps. So it would be in a context where someone is modifying and re-testing their app.

That's correct. In the ASP.NET runtime, we explicitly opted to keep these attributes in existing code paths, even though our policy was to avoid adding them to new code paths. The reason for this is that even though we publicly messaged that partial trust was dead (as a security boundary), we didn't want to risk breaking existing applications that were already deployed in partial trust environments.

For third-party library vendors, I guess the recommendation would be that if you're servicing an existing library, keep the existing annotations to avoid inadvertently breaking customers. But if you're introducing a new version of the library, remove the annotations.

[danmoseley]

OK. @tdykstra I suggest we proceed with this PR 😄

[tdykstra]

If the consensus is to proceed with this PR, I need someone to approve it. I'll continue with more batches like this one but won't make changes in files where the only CAS presence is a using directive.

[danmoseley]

@GrabYourPitchforks sounds like you already reviewed?

[danmoseley]

@GrabYourPitchforks did you already review? If so perhaps we can sign off and get this merged?

[tdykstra]

Discussed this PR offline with @GrabYourPitchforks and @danmosemsft and we settled on a different approach: Levi will identify specific categories of changes required, and for each category I'll make a PR that addresses all affected snippets. I'm closing this PR since it started on a different approach, i.e., trying to include all categories of CAS-related changes in each PR, with each PR addressing a subset of affected snippets.