Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 18 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions aspnetcore/includes/managed-identities-test-non-production.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
> [!NOTE]
> Microsoft recommends that you use the most secure authentication flow available. The authentication flow described in this procedure is only suitable for testing and local development.
>
> Azure SQL Database should use [Managed Identities for Azure resources](/sql/connect/ado-net/sql/azure-active-directory-authentication#using-managed-identity-authentication) instead of the flow described in this tutorial. For non-Azure apps, use a secure authentication flow similar to [Managed Identities for Azure resources](/sql/connect/ado-net/sql/azure-active-directory-authentication#using-managed-identity-authentication).
wadepickett marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 4 additions & 2 deletions aspnetcore/tutorials/razor-pages/sql.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ title: Part 4, work with a database
author: wadepickett
description: Part 4 of tutorial series on Razor Pages.
ms.author: wpickett
ms.date: 06/23/2024
ms.date: 08/20/2024
uid: tutorials/razor-pages/sql
---
# Part 4 of tutorial series on Razor Pages
Expand Down Expand Up @@ -40,7 +40,9 @@ The generated connection string is similar to the following JSON:

---

When the app is deployed to a test or production server, an environment variable can be used to set the connection string to a test or production database server. For more information, see [Configuration](xref:fundamentals/configuration/index).
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).

[!INCLUDE [managed-identities](~/includes/managed-identities-test-non-production.md)]

# [Visual Studio](#tab/visual-studio)

Expand Down
4 changes: 3 additions & 1 deletion aspnetcore/tutorials/razor-pages/sql/includes/sql3.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ The generated connection string will be similar to the following:

---

When the app is deployed to a test or production server, an environment variable can be used to set the connection string to a test or production database server. See [Configuration](xref:fundamentals/configuration/index) for more information.
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).
Copy link
Contributor

@Rick-Anderson Rick-Anderson Aug 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@wadepickett wadepickett Aug 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"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.

Copy link
Contributor Author

@wadepickett wadepickett Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@Rick-Anderson Rick-Anderson Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


[!INCLUDE [managed-identities](~/includes/managed-identities-test-non-production.md)]

# [Visual Studio](#tab/visual-studio)

Expand Down
4 changes: 3 additions & 1 deletion aspnetcore/tutorials/razor-pages/sql/includes/sql5.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ The generated connection string is similar to the following JSON:

---

When the app is deployed to a test or production server, an environment variable can be used to set the connection string to a test or production database server. For more information, see [Configuration](xref:fundamentals/configuration/index).
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).

[!INCLUDE [managed-identities](~/includes/managed-identities-test-non-production.md)]

# [Visual Studio](#tab/visual-studio)

Expand Down
4 changes: 3 additions & 1 deletion aspnetcore/tutorials/razor-pages/sql/includes/sql6.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ The generated connection string is similar to the following JSON:

---

When the app is deployed to a test or production server, an environment variable can be used to set the connection string to a test or production database server. For more information, see [Configuration](xref:fundamentals/configuration/index).
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).

[!INCLUDE [managed-identities](~/includes/managed-identities-test-non-production.md)]

# [Visual Studio](#tab/visual-studio)

Expand Down
4 changes: 3 additions & 1 deletion aspnetcore/tutorials/razor-pages/sql/includes/sql7.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ The generated connection string is similar to the following JSON:

---

When the app is deployed to a test or production server, an environment variable can be used to set the connection string to a test or production database server. For more information, see [Configuration](xref:fundamentals/configuration/index).
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).

[!INCLUDE [managed-identities](~/includes/managed-identities-test-non-production.md)]

# [Visual Studio](#tab/visual-studio)

Expand Down
4 changes: 3 additions & 1 deletion aspnetcore/tutorials/razor-pages/sql/includes/sql8.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ The generated connection string is similar to the following JSON:

---

When the app is deployed to a test or production server, an environment variable can be used to set the connection string to a test or production database server. For more information, see [Configuration](xref:fundamentals/configuration/index).
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).

[!INCLUDE [managed-identities](~/includes/managed-identities-test-non-production.md)]

# [Visual Studio](#tab/visual-studio)

Expand Down
Loading