ROPC: include for secure auth flow - managed identities - Razor Pages Tutorial

[wadepickett]

Fixes #33407
Contributes to #33407

• Created include:
  includes/managed-identities-test-non-production.md

  Using standard note template, we are all using for ROPC.

  See similar related include used by dotnet/docs:
  SFI - ROPC: First group of updates docs#42254

  This note was created in line with a recommended examples in the contributor's guide for test / non-production scenarios here:
  https://review.learn.microsoft.com/en-us/help/platform/contribute-security-insecure-auth-flows?branch=main

• Updated the Razor Pages Tutorial series to add the include and other recommendations related to the example of connecting to a database created through Entity Framework from a model class.

---

Internal previews

📄 File	🔗 Preview link
aspnetcore/tutorials/razor-pages/sql.md	Part 4 of tutorial series on Razor Pages

[wadepickett]

close open and rebuild, include name change was not registering.

[wadepickett]

@tdykstra, @Rick-Anderson does this seem like the right approach for the tutorials where db connection strings are involved but for a non-production tutorial teaching situation like these? I can also take the feedback offline.

[Rick-Anderson]

Too weak for ASP.NET Core. Stronger recommendations needed.
Our tutorials don't and shouldn't focus on secure auth, link off to that.

[Rick-Anderson]

The preceding needs to be fixed. Only test should use the env variable.

builder.Services.AddDbContext<RazorPagesMovieContext>(options =>
    options.UseSqlServer(builder.Configuration.GetConnectionString("RazorPagesMovieContext") ?? throw new InvalidOperationException("Connection string 'RazorPagesMovieContext' not found.")));

The preceding code is not insecure.

[Rick-Anderson]

Suggested change

	When the app is deployed to a test, non-production server, an environment variable can be used to set the connection string to a test, non-production database server. For more information, see [Configuration](xref:fundamentals/configuration/index).
	In this tutorial, a local database that doesn't need a password is used. Therefore, a secure authentication flow isn't required.
	When the app is deployed to a test server, an environment variable can be used to set the connection string to point to a test database server. An environment variable should not be used in a production system. For more information, see [Configuration](xref:fundamentals/configuration/index).

@tdykstra I think that's easier to read.

[Rick-Anderson]

@tdykstra I think we should put the above in an include file and use it everywhere. Then we only have one place to change it. What do you think?

We'll need this boilerplate in MVC and other place. We could add it to EF tutorials.

[Rick-Anderson]

@wadepickett @tdykstra we need to work in something like this.

For VS/local db:
In this tutorial, localdb is used in dev on the local machine and doesn't use a password, therefore, a secure authentication flow is not required.

Then something similar for SqlLite

[Rick-Anderson]

We don't want to spook them right away so we need to tell them up front that since this tutorial uses a local db that doesn't use a password, a secure authentication flow isn't required. Maybe tell them that before the big warning.

[wadepickett]

@Rick-Anderson, sure we can create another include for that case too only for VS/localDB

Clarification/question: For VS/localDB, when Trusted_Connection=True as we are using in the connect here, that results in Windows Authentication used. Wouldn't we consider that a type of Authentication flow? So to say one isn't required might be confusing. It may not be the best one for the situation depending, but is one. Maybe we are keeping to a narrower definition of Authentication flow?

[Rick-Anderson]

@Rick-Anderson, sure we can create another include for that case too only for VS/localDB

Clarification/question: For VS/localDB, when Trusted_Connection=True as we are using in the connect here, that results in Windows Authentication used. Wouldn't we consider that a type of Authentication flow? So to say one isn't required might be confusing. It may not be the best one for the situation depending, but is one. Maybe we are keeping to a narrower definition of Authentication flow?

Resource Owner Password Credentials (ROPC) grant is using a PW. No PW is used with local DB. We're only concerned where a PW is used, like the old deployment method where we put the PW in an env variable.

[tdykstra]

@tdykstra I think that's easier to read.

Yes, and I tweaked it a bit.

@tdykstra I think we should put the above in an include file and use it everywhere. Then we only have one place to change it. What do you think?

Sounds good, but I don't think we should say "don't use environment variables in production" without saying what is OK to use and providing a link to how-to instructions.

[wadepickett]

"Then something similar for SqlLite"

@Rick-Anderson, I think the way you worded it in the last edit already fits both situations nicely by specifying "a local database" rather than more specifically "localDB".

"In this tutorial, a local database that doesn't need a password is used."

So only the one include would be needed for the note on using a local db. I will just add one include with your text, unless you still want some sort of separate message for SQLite.

[wadepickett]

Sounds good, but I don't think we should say "don't use environment variables in production" without saying what is OK to use and providing a link to how-to instructions.

@tdykstra, to address this I added "An environment variable should not be used in a production system. For a production system, use a secure method such as Azure Key Vault configuration provider or Managed Identities for Azure resources to manage sensitive information like connection strings."

See the updated include. If they are not using Azure SQL server, then it would have to be key vault I assume, if they are then it would be Managed Identities for Azure resources.

However, between both includes it feels a little duplicated so maybe I need to address it differently in a combined way. See what you think.

[Rick-Anderson]

However, between both includes it feels a little duplicated so maybe I need to address it differently in a combined way. See what you think.

Yup, too long, too much duplication.

[wadepickett]

Moved Rick's suggestion on a local db with no password info to an include and applied for Razor Pages example.

Also addressed Tom's suggestion on recommending the correct path for production since we say env var cannot be used.

All suggestions I know of are in. Good to know what you think of the added include Rick suggested, plus the change to address Tom's concern specify what to do instead of env var in production.

[Rick-Anderson]

The following is WAY TO LONG

It repeats too much. @tdykstra how is my suggestion?

[Rick-Anderson]

@wadepickett do a pull before you push as I committed @tdykstra change, it was much better.

[Rick-Anderson]

@wadepickett create an issue to create [Secure authentication flows](/aspnet/core/security/<!-- write create article something like secureAuthn-->).

[wadepickett]

Just to make sure I am in sync with the intent:

• You added the commit that refers to a future topic not created so it is in the commit history along with Tom's last suggestion for it.
• Then you want me to pave over that with the last longer version of the include to merge today?

[wadepickett]

Tracking issue: #33426 Secure Authentication Flows

[wadepickett]

@Rick-Anderson, The last commit you added had a non-existent link. Did you mean to change the include first? This is why I was asking that, if for now, we were using the previous version of the include before your last commit. Here is what it looks like now:

[wadepickett]

Please don't merge my PR's for me, eh? I would rather do that.

[Rick-Anderson]

Oops,